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

Look at response-headers in addition to request-headers/query-params for ModPagespeed=* options #533

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 14 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter

GoogleCodeExporter commented Apr 6, 2015

This will allow sites to control mod_pagespeed operation from content 
generation flow. 

Original issue reported on code.google.com by jmara...@google.com on 16 Oct 2012 at 10:23

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Issue 518 has been merged into this issue.

Original comment by jkar...@google.com on 26 Oct 2012 at 3:34

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jkar...@google.com on 26 Oct 2012 at 3:34

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jkar...@google.com on 31 Oct 2012 at 5:34

  • Added labels: Milestone-v23
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by jkar...@google.com on 31 Oct 2012 at 5:34

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

This does not appear to work with the Worker MPM.  At least the system test 
fails.

TEST: Request Headers affect MPS options
 TEST: ModPagespeed: off header was stripped
     check_not fgrep ModPagespeed: off /tmp/mod_pagespeed_test.jmarantz/fetched_directory/result_off
  ModPagespeed: off
     failure at line 154
FAIL.

Original comment by jmara...@google.com on 1 Nov 2012 at 2:43

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Found the problem. We need to process both record->headers_out and 
record->err_headers_out (which is where fastcgi sticks php's output) and gets 
merged after mod_pagespeed runs.

Original comment by jkar...@google.com on 1 Nov 2012 at 7:08

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Another comment:  mod_pagespeed currently ignores response-header
  Cache-Control: no-transform
See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9.5

Treating this like cache-control:private would make mod_pagespeed correct and 
partially address this issue.

Another option would be to still allow cache-extension of images, js, and some 
CSS with CC:no-transform, keeping that response-header.  CSS is a little 
trickier as if we are rewriting its domain or path then we must transform its 
contents to fix URLs, so we'd only be able to cache-extend CSS that we weren't 
going to move.

Original comment by jmara...@google.com on 12 Nov 2012 at 9:14

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

It doesn't seem worth cluttering up the code with guards unless we find that 
no-transform is common enough that we're losing a lot of caching opportunity.  
Further, there is risk of us getting it wrong.  Therefore I'm in favor of 
treating it like cache-control:private.

Original comment by jkar...@google.com on 4 Dec 2012 at 12:53

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

mod_pagespeed will also need to obey no-transform for HTML.

Original comment by jkar...@google.com on 4 Dec 2012 at 1:11

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

[deleted comment]
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

To look for no-transform in HTML we'll need to look at the response headers as 
well as for meta http-equiv tags in the HTML itself.

Original comment by jkar...@google.com on 4 Dec 2012 at 1:15

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

I'm not convinced we need to look at <meta http-equiv> tags for no-transform. 
Proxies should be able to operate without parsing HTML (being purely HTTP 
proxies). Either way, I think this is a separate issue.

Original comment by sligocki@google.com on 4 Dec 2012 at 7:18

@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Closing this as we do look at ModPagespeed=* options now in response headers, 
and respecting Cache-Control: no-transform is ongoing as a separate issue.

Original comment by jkar...@google.com on 10 Dec 2012 at 8:04

  • Changed state: Fixed
  • Added labels: Milestone-v24
  • Removed labels: Milestone-v23
@GoogleCodeExporter

This comment has been minimized.

GoogleCodeExporter commented Apr 6, 2015

Original comment by matterb...@google.com on 10 Dec 2012 at 8:30

  • Added labels: release-note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment