View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0011915 | mantisbt | attachments | public | 2010-05-11 12:22 | 2015-05-01 01:51 |
Reporter | klkl | Assigned To | |||
Priority | normal | Severity | feature | Reproducibility | N/A |
Status | new | Resolution | open | ||
Product Version | git trunk | ||||
Summary | 0011915: Fixed cacheability of attachments | ||||
Description | I've refactored file_download.php a bit to reintroduce allow_file_cache config option without causing trouble in IE+HTTPS. I've also added very basic support for If-Modified-Since header. This is huge improvement for image previews displayed inline (they're not re-downloaded over and over again). | ||||
Tags | patch | ||||
Attached Files | attachment_cache.patch (7,610 bytes)
From 43b4e0d82fae524454aa01816dcf0a60860d4609 Mon Sep 17 00:00:00 2001 From: Kornel Lesinski <kornel@aardvarkmedia.co.uk> Date: Tue, 11 May 2010 15:36:23 +0100 Subject: [PATCH 1/5] Buffer flush without error suppresion. --- file_download.php | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/file_download.php b/file_download.php index d2101eb..783b5e1 100644 --- a/file_download.php +++ b/file_download.php @@ -35,7 +35,9 @@ * @uses utility_api.php */ +global $g_bypass_headers; $g_bypass_headers = true; # suppress headers as we will send our own later + define( 'COMPRESSION_DISABLED', true ); require_once( 'core.php' ); @@ -104,7 +106,9 @@ switch ( $f_type ) { } # throw away output buffer contents (and disable it) to protect download -while ( @ob_end_clean() ); +while ( ob_get_level() ) { + ob_end_clean(); +} if ( ini_get( 'zlib.output_compression' ) && function_exists( 'ini_set' ) ) { ini_set( 'zlib.output_compression', false ); -- 1.7.0.2 From a2abd3e0399260b7816123e6868298af25d8a862 Mon Sep 17 00:00:00 2001 From: Kornel Lesinski <kornel@aardvarkmedia.co.uk> Date: Tue, 11 May 2010 15:19:41 +0100 Subject: [PATCH 2/5] Added support for bypass_headers flag to session API to prevent PHP's lame session headers from interfering with file downloads. --- core/session_api.php | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/core/session_api.php b/core/session_api.php index f4cfbcb..79a55e2 100644 --- a/core/session_api.php +++ b/core/session_api.php @@ -101,6 +101,7 @@ class MantisPHPSession extends MantisSession { function __construct( $p_session_id=null ) { global $g_cookie_secure_flag_enabled; global $g_cookie_httponly_flag_enabled; + global $g_bypass_headers; $this->key = config_get_global( 'session_key' ); @@ -111,7 +112,14 @@ class MantisPHPSession extends MantisSession { } # Handle session cookie and caching - session_cache_limiter( 'private_no_expire' ); + if ( empty($g_bypass_headers) ) { + session_cache_limiter( 'private_no_expire' ); + } else { + # File downloads require special cache handling + # 'none' makes PHP allow custom headers + session_cache_limiter( 'none' ); + } + if ( $g_cookie_httponly_flag_enabled ) { # The HttpOnly cookie flag is only supported in PHP >= 5.2.0 session_set_cookie_params( 0, config_get( 'cookie_path' ), config_get( 'cookie_domain' ), $g_cookie_secure_flag_enabled, $g_cookie_httponly_flag_enabled ); -- 1.7.0.2 From 6fb80875d8d2c970c971e607dd81e6536061f1ff Mon Sep 17 00:00:00 2001 From: Kornel Lesinski <kornel@aardvarkmedia.co.uk> Date: Tue, 11 May 2010 15:39:27 +0100 Subject: [PATCH 3/5] Made IE workaround separate from allow_file_cache configuration setting. --- config_defaults_inc.php | 6 +++--- file_download.php | 21 ++++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/config_defaults_inc.php b/config_defaults_inc.php index f852da3..e956173 100644 --- a/config_defaults_inc.php +++ b/config_defaults_inc.php @@ -3169,13 +3169,13 @@ $g_custom_headers = array(); * @global int $g_allow_browser_cache */ // $g_allow_browser_cache = ON; + /** * File caching - This will allow the browser to cache downloaded files. - * Without this set, there may be issues with IE receiving files, and launching - * support programs. + * This setting is ignored for IE to work around its bugs. * @global int $g_allow_file_cache */ - // $g_allow_file_cache = ON; +$g_allow_file_cache = ON; /***************** * Custom Fields * diff --git a/file_download.php b/file_download.php index 783b5e1..e37a6f6 100644 --- a/file_download.php +++ b/file_download.php @@ -114,21 +114,24 @@ if ( ini_get( 'zlib.output_compression' ) && function_exists( 'ini_set' ) ) { ini_set( 'zlib.output_compression', false ); } -# Make sure that IE can download the attachments under https. -header( 'Pragma: public' ); +$t_allow_file_cache = config_get( 'allow_file_cache' ); +$t_using_https = isset( $_SERVER["HTTPS"] ) && ( "on" == strtolower( $_SERVER["HTTPS"] ) ); # To fix an IE bug which causes problems when downloading # attached files via HTTPS, we disable the "Pragma: no-cache" # command when IE is used over HTTPS. -global $g_allow_file_cache; -if ( ( isset( $_SERVER["HTTPS"] ) && ( "on" == utf8_strtolower( $_SERVER["HTTPS"] ) ) ) && is_browser_internet_explorer() ) { - # Suppress "Pragma: no-cache" header. +if ( $t_using_https && is_browser_internet_explorer() ) { + $t_allow_file_cache = ON; +} + +if ( ON === $t_allow_file_cache ) { + header( 'Pragma: public' ); + header( 'Cache-Contorl: public' ); } else { - if ( !isset( $g_allow_file_cache ) ) { - header( 'Pragma: no-cache' ); - } + header( 'Expires: ' . gmdate( DATE_RFC1123, time() ) ); + header( 'Pragma: no-cache' ); + header( 'Cache-Control: no-cache' ); } -header( 'Expires: ' . gmdate( 'D, d M Y H:i:s \G\M\T', time() ) ); header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s \G\M\T', $v_date_added ) ); -- 1.7.0.2 From 0149b39f22804d7bbbb9a95a19a16d7992a72434 Mon Sep 17 00:00:00 2001 From: Kornel Lesinski <kornel@aardvarkmedia.co.uk> Date: Tue, 11 May 2010 15:37:25 +0100 Subject: [PATCH 4/5] Don't set must-revalidate and Expires when allow_caching is ON. --- core/http_api.php | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/core/http_api.php b/core/http_api.php index ada3b80..12e3f69 100644 --- a/core/http_api.php +++ b/core/http_api.php @@ -101,16 +101,12 @@ function http_caching_headers( $p_allow_caching=false ) { // with option to bypass if running from script if ( !headers_sent() ) { if ( $p_allow_caching || ( isset( $g_allow_browser_cache ) && ON == $g_allow_browser_cache ) ) { - if ( is_browser_internet_explorer() ) { - header( 'Cache-Control: private, proxy-revalidate' ); - } else { - header( 'Cache-Control: private, must-revalidate' ); - } + header( 'Cache-Control: private' ); } else { - header( 'Cache-Control: no-store, no-cache, must-revalidate' ); + header( 'Expires: ' . gmdate( 'D, d M Y H:i:s \G\M\T', time() ) ); + header( 'Cache-Control: no-cache, must-revalidate' ); } - header( 'Expires: ' . gmdate( 'D, d M Y H:i:s \G\M\T', time() ) ); header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s \G\M\T', time() ) ); } } -- 1.7.0.2 From c90293db3962fb881a33ba98f669d93c0c1e7c49 Mon Sep 17 00:00:00 2001 From: Kornel Lesinski <kornel@aardvarkmedia.co.uk> Date: Tue, 11 May 2010 15:41:05 +0100 Subject: [PATCH 5/5] Added basic If-Modified-Since support. --- file_download.php | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/file_download.php b/file_download.php index e37a6f6..b9d062b 100644 --- a/file_download.php +++ b/file_download.php @@ -133,7 +133,16 @@ if ( ON === $t_allow_file_cache ) { header( 'Cache-Control: no-cache' ); } -header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s \G\M\T', $v_date_added ) ); +$t_last_modified_date = gmdate( DATE_RFC1123, $v_date_added ); + +header( 'Last-Modified: ' . $t_last_modified_date ); + +# Support HTTP/1.1 cache validation +#@@@ This is oversimplified implementation that does not parse and compare dates, and relies on browser using exactly the same date format. +if ( isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) && $_SERVER['HTTP_IF_MODIFIED_SINCE'] === $t_last_modified_date ) { + header('HTTP/1.1 304 Not Modified'); + die(); +} $t_filename = file_get_display_name( $v_filename ); $t_show_inline = false; -- 1.7.0.2 | ||||
+global $g_bypass_headers; Isn't $g_bypass_headers in that context within global scope? I though you only need to declare a variable as global from within a lesser scope (such as a function)? |
|
Yes, indeed. It's just a defensive habit I've got from projects that do include files in random places. |
|