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. |
|