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

FIX: Remove X-Requested-With from default Vary header. #8242

Merged
merged 3 commits into from Jul 5, 2018

Conversation

sminnee
Copy link
Member

@sminnee sminnee commented Jul 4, 2018

The X-Requested-With header does modify the result of Director::is_ajax
and so this should strictly be in there. In practise, this can cause
issues with CDNs such as Incapsula, and LeftAndMain adds this vary
header itself, which is the principle place where Director::is_ajax
is used.

The X-Requested-With header does modify the result of Director::is_ajax
and so this should strictly be in there. In practise, this can cause
issues with CDNs such as Incapsula, and LeftAndMain adds this vary
header itself, which is the principle place where Director::is_ajax
is used.
@chillu
Copy link
Member

chillu commented Jul 4, 2018

+1 on this, but should be accompanied by a PHPDoc note in is_ajax() as well as a note in our "HTTP headers" markdown docs.

@sminnee
Copy link
Member Author

sminnee commented Jul 4, 2018

How's that @chillu ?

@chillu chillu self-assigned this Jul 4, 2018
@chillu chillu merged commit 13e8b15 into silverstripe:3.7 Jul 5, 2018
@chillu chillu deleted the simpler-vary-header branch July 5, 2018 01:32
@chillu
Copy link
Member

chillu commented Jul 17, 2018

Note that this won't be part of the 3.7.1 and 4.2.0 releases (those have been tagged weeks ago for a security audit), we'll need to do follow up patch releases to get this out.

@chillu
Copy link
Member

chillu commented Jul 23, 2018

Forward ported this to 4.2: #8280

I've changed my mind about including this in 4.2.0: I think it's crucial in order to explain caching behaviour and API changes consistently in both the release notes and updated caching docs (CWP and docs.ss.org). To this end, I have added a task for @dhensby on your internal release checklists in https://github.com/silverstripeltd/cc-issues/issues/163 to cherry pick this for both the 3.7.0 and 4.2.0 releases.

dhensby pushed a commit to open-sausages/silverstripe-framework that referenced this pull request Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants