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.

Sign up for GitHub

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

Jump to bottom

Rename default header name to prevent being cut by web-servers #120

Closed
georgique opened this issue Feb 4, 2020 · 3 comments
Closed

Rename default header name to prevent being cut by web-servers #120

georgique opened this issue Feb 4, 2020 · 3 comments
Assignees
@mebjas
Labels
next ver
Milestone
Version 1.1.0

Comments

@georgique
Copy link

Although RFC doesn't prohibit it, using underscores in header names is not common. And for example Nginx by default considers such headers as invalid unless underscore_in_headers param is set to "on".
I suggest to make default header name "CSRF-Token" so it looks more consistent with standard header names and doesn't cause troubles with web-servers.
@mebjas
Copy link
Owner

mebjas commented Feb 6, 2020

Sounds good to me. Would you be interested in sending a PR with fix?

@mebjas mebjas self-assigned this Feb 6, 2020
@mebjas mebjas added the next ver label Feb 8, 2020
@mebjas mebjas added this to the Version 1.1.0 milestone Feb 8, 2020
@polishdeveloper
Copy link
Contributor

Just highlighting that this is pretty important issue. Apache 2.4 also removes that header (http://httpd.apache.org/docs/trunk/env.html - see "Passing broken headers to CGI scripts" section)

polishdeveloper added a commit to polishdeveloper/CSRF-Protector-PHP that referenced this issue May 14, 2020
@polishdeveloper
Fix mebjas#120 by renaming the csrfp-token to conform to header rules
8a14cf3 
Both Nginx and Apache2.4 treat the headers with underscore as invalid.
As a solution lets rename the token name to CSRFP-Token that should be
valid for cookie,get and headers.

Issue: mebjas#120
@polishdeveloper polishdeveloper mentioned this issue May 14, 2020
Merged
@mebjas mebjas closed this as completed in 1c5f157 May 15, 2020
mebjas added a commit that referenced this issue May 15, 2020
@mebjas
Merge pull request #123 from polishdeveloper/token_rename
563132a 
Fix #120 by renaming the csrfp-token to conform to header rules
@mebjas
Copy link
Owner

mebjas commented May 15, 2020

Thanks for calling this out - @georgique and thanks for the fix @polishdeveloper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mebjas mebjas

Labels
next ver
Projects
None yet
Milestone
Version 1.1.0
Development

No branches or pull requests
3 participants
@mebjas @polishdeveloper @georgique