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

Default FPC Accepts non text/html, Serves Cache Hit as text/html #3845

Closed
astorm opened this issue Mar 20, 2016 · 6 comments
Closed

Default FPC Accepts non text/html, Serves Cache Hit as text/html #3845

astorm opened this issue Mar 20, 2016 · 6 comments
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@astorm
Copy link

astorm commented Mar 20, 2016

The default full page cache (FPC) implementation will cache content with non html Content-Type headers (text/css, application/javascript, etc), and then re-serve the content as text/html. This can cause unstable and unacceptable application behavior. For example, Google Chrome will not render CSS files served with a text/html header.

This is not a theoretical problem. It was encountered when converting an business critical feature for a popular Magento 1 theme to a Magento 2 theme. While suggestions for better coding practices are appreciated, the shipping full page cache engine should not ship with functionality that's broken.

Possible fixes include ensuring the FPC engine skips caching non text/html content, or ensuring the FPC engine preserves header. This is a decision that needs to be made by the Magento core engineering team as it has long term implications for the behavior of the platform.

Steps to reproduce

  1. Install Magento from develop branch.
  2. Ensure Full Page Cache is Enabled at System -> Cache Management
  3. Install Attached Module (Pulsestorm_BugFullPageCacheHeader.tar.gz
    )
  4. Load URL: http://magento.example.com/pulsestorm_bugfullpagecacheheader/
  5. Reload URL: http://magento.example.com/pulsestorm_bugfullpagecacheheader/

Expected result

  1. Both results return a text/css Content-Type: header

Actual result

  1. Second Result returns a text/html header

Curl output below

$ curl '-I' 'http://magento-2-with-keys.dev/pulsestorm_bugfullpagecacheheader/'
HTTP/1.1 200 OK
Date: Sun, 20 Mar 2016 18:41:07 GMT
Server: Apache/2.4.16 (Unix) PHP/5.6.19
X-Powered-By: PHP/5.6.19
Expires: Fri, 20 Mar 2015 18:41:08 GMT
Cache-Control: max-age=0, must-revalidate, no-cache, no-store
Pragma: no-cache
X-Magento-Cache-Control: max-age=86400, public, s-maxage=86400
X-Magento-Cache-Debug: MISS
X-Frame-Options: SAMEORIGIN
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Content-Type: text/css;charset=UTF-8

Alans-MacBook-Pro:magento2 alanstorm$ curl '-I' 'http://magento-2-with-keys.dev/pulsestorm_bugfullpagecacheheader/'
HTTP/1.1 200 OK
Date: Sun, 20 Mar 2016 18:41:09 GMT
Server: Apache/2.4.16 (Unix) PHP/5.6.19
X-Powered-By: PHP/5.6.19
X-Magento-Cache-Debug: HIT
X-Frame-Options: SAMEORIGIN
X-Content-Type-Options: nosniff
X-XSS-Protection: 1; mode=block
Content-Type: text/html; charset=UTF-8
@alankent
Copy link

That's a pretty clear bug.

@choukalos
Copy link

Created bug MAGETWO-50710 for internal tracking

@sshrewz sshrewz added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Mar 25, 2016
@astorm
Copy link
Author

astorm commented May 5, 2016

Any update/ETA on this? It's causing a real world problem for an extension/theme developer who's migrating their theme to Magento 2.

@astorm
Copy link
Author

astorm commented May 9, 2016

Any update/ETA on this? It's causing a real world problem for an extension/theme developer who's migrating their theme to Magento 2.

/cc @piotrekkaminski

@pboisvert
Copy link

@choukalos Can you update this issue? Jira shows as closed now but unsure when that ticket will be released (2.1 or earlier in a patch).

@piotrekkaminski
Copy link
Contributor

The issue looks fixed in internal repository and should be available already in 2.1rc versions.

magento-engcom-team pushed a commit that referenced this issue Mar 11, 2019
[tango] MAGETWO-98409: Config:set command failed for path bigger than 3 depth
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

6 participants