View Issue Details

IDProjectCategoryView StatusLast Update
0011915mantisbtattachmentspublic2015-05-01 01:51
Reporterklkl Assigned To 
PrioritynormalSeverityfeatureReproducibilityN/A
Status newResolutionopen 
Product Versiongit trunk 
Summary0011915: 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).

Tagspatch
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

attachment_cache.patch (7,610 bytes)   

Relationships

has duplicate 0012926 closedatrol Attached images are always realoaded 

Activities

dhx

dhx

2010-05-11 23:45

reporter   ~0025480

+global $g_bypass_headers;
$g_bypass_headers = true; # suppress headers as we will send our own later
+

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)?

klkl

klkl

2010-05-12 04:59

reporter   ~0025484

Yes, indeed. It's just a defensive habit I've got from projects that do include files in random places.