Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Pragma headers in file_download.php #1950

Merged
merged 4 commits into from Dec 10, 2023

Conversation

dregad
Copy link
Member

@dregad dregad commented Dec 2, 2023

Fixes #33007

@dregad dregad self-assigned this Dec 2, 2023
@atrol
Copy link
Member

atrol commented Dec 2, 2023

As this is the only place where $g_allow_file_cache is used, you can also remove it from config_defaults_inc.php and the documentation.

@vboctor
Copy link
Member

vboctor commented Dec 2, 2023

I was working on the caching to leverage the fact that attachments are never updated and came up with the following which reduces requests to server and works really well:

$t_last_modified_time = $v_date_added;
$t_etag = md5( $t_last_modified_time );

header( 'Cache-Control: private, max-age=31536000' );
header( 'Expires: ' . gmdate( 'D, d M Y H:i:s \G\M\T', time() + 31536000 ) );
header( 'ETag: ' . $t_etag );
header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s \G\M\T', $t_last_modified_time ) );
header( 'Pragma: cache' );

# This is needed to avoid Mantis preparing the payload and then the http server not sending it to the client
if ( ( isset( $_SERVER['HTTP_IF_NONE_MATCH'] ) && trim( $_SERVER['HTTP_IF_NONE_MATCH'] ) == $t_etag ) ||
     ( isset( $_SERVER['HTTP_IF_MODIFIED_SINCE'] ) && strtotime( $_SERVER['HTTP_IF_MODIFIED_SINCE'] ) == strtotime( $t_last_modified_time ) ) ) {
    header('HTTP/1.1 304 Not Modified');
    exit;
}

@dregad
Copy link
Member Author

dregad commented Dec 3, 2023

As this is the only place where $g_allow_file_cache is used, you can also remove it from config_defaults_inc.php and the documentation.

@atrol done

@dregad
Copy link
Member Author

dregad commented Dec 3, 2023

I was working on the caching to leverage the fact that attachments are never updated and came up with the following which reduces requests to server and works really well:

@vboctor I'm fine with that although I did not actually test it, but this change is outside of this PR's scope so please open a new issue on the tracker and a separate PR with your proposed fix.

@vboctor
Copy link
Member

vboctor commented Dec 4, 2023

@dregad I just did some testing, it works well for the download scenario, but not the preview scenario, since a unique security token is added to the preview URLs, which cause it not to cache. For the preview scenario to work, we will have to have an alternative for file_show_inline_token query string.

I agree it is outside the scope of this PR.

@dregad
Copy link
Member Author

dregad commented Dec 10, 2023

@atrol without feedback from you since I pushed it last week, I assume you're OK with the commit removing allow_file_cache config.

It is a deprecated HTTP/1.0 thing that should be replaced with a
Cache-Control header. Additionally we use it in an incorrect way, as the
directive is only defined in a request context (not response).

This was implemented as a workaround for an Internet Explorer bug, and
we don't support this browser anymore.

Fixes #33007
It was used for Internet Explorer compatibility, and we don't support
this browser anymore.

Fixes #33007
@dregad dregad merged commit 10f9816 into mantisbt:master Dec 10, 2023
1 check failed
@dregad dregad deleted the i33007-remove-pragma branch December 10, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants