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

Application handling of field names #844

Merged
merged 7 commits into from May 27, 2021
Merged

Application handling of field names #844

merged 7 commits into from May 27, 2021

Conversation

mnot
Copy link
Member

@mnot mnot commented May 20, 2021

Fixes #843

@mnot mnot requested review from reschke and royfielding May 20, 2021 05:07
reschke
reschke requested changes May 20, 2021
View reviewed changes
draft-ietf-httpbis-semantics-latest.xml Outdated Show resolved Hide resolved
@mnot @reschke
Update draft-ietf-httpbis-semantics-latest.xml
ca44bec 
Co-authored-by: Julian Reschke <julian.reschke@gmx.de>
royfielding
royfielding reviewed May 26, 2021
View reviewed changes
Copy link
Member

@royfielding royfielding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to clean up the comments to improve the text.

draft-ietf-httpbis-semantics-latest.xml Outdated Show resolved Hide resolved
royfielding
royfielding requested changes May 26, 2021
View reviewed changes
Copy link
Member

@royfielding royfielding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my suggested rewrite.

draft-ietf-httpbis-semantics-latest.xml Outdated Show resolved Hide resolved
@royfielding royfielding requested a review from reschke May 26, 2021 17:15
reschke
reschke approved these changes May 26, 2021
View reviewed changes
draft-ietf-httpbis-semantics-latest.xml Outdated Show resolved Hide resolved
@royfielding
Copy link
Member

royfielding commented May 26, 2021 •
edited

section 4.1.8 is better as a reference

https://datatracker.ietf.org/doc/html/rfc3875#section-4.1.18

though it reminds me that CONTENT_LENGTH is already protected, so our example is wrong. Maybe we should change it to a Transfer-Encoding example, but we can't do that because that's only defined in 1.1.

@royfielding
Copy link
Member

The interface maps the Content-Length header field to CONTENT_LENGTH, not HTTP_CONTENT_LENGTH, and CGI-based applications will always look to the CONTENT_LENGTH value regardless of the presence of HTTP_CONTENT_LENGTH.

@royfielding
Copy link
Member

Though, if someone were foolish enough to write an HTTP-CGI-HTTP gateway, all bets are off.

@royfielding
Copy link
Member

I edited my suggestion again to be more specific about CGI and CGI legacy, and to fix the incorrect example of CONTENT_LENGTH. This may require adding an informative reference for RFC3875.

@reschke reschke merged commit 96be598 into master May 27, 2021
@reschke reschke deleted the mnot-843 branch May 27, 2021 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk left review comments

@royfielding royfielding royfielding requested changes

@reschke reschke reschke approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Security considerations: _ in field names
4 participants
@mnot @royfielding @reschke @annevk