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

Enhances validation of HTTP header names #2212

Open
carryel opened this issue Sep 13, 2024 · 0 comments
Open

Enhances validation of HTTP header names #2212

carryel opened this issue Sep 13, 2024 · 0 comments

Comments

@carryel
Copy link
Contributor

carryel commented Sep 13, 2024

RFC 9110 specifies that only the following characters are allowed within header names:

field-name     = token
token          = 1*tchar
tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                 / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                 / DIGIT / ALPHA
                 ; any VCHAR, except delimiters

Grizzly HTTP does not enforce this rule.

1. I think Grizzly should follow this convention for header names.

Consider the following payload: GET / HTTP/1.1\r\nHost: a\r\nIgnore\r\nMy-Header: m\r\n\r\n.

Grizzly's HTTP parser sees this payload as two requests, like so:

GET / HTTP/1.1\r\n
Host: a\r\n
Ignore\r\nMy-Header: m\r\n
\r\n

However, some HTTP parsers (e.g. Nginx, Libsoup, cpp-httplib) see only request, like so:

GET / HTTP/1.1\r\n
Host: a\r\n
Ignore\r\n
My-Header: m\r\n
\r\n

This happens because these parsers either ignore or permissively parse field-lines with no ':', so they see a My-Header header where Grizzly didn't.

2. I think it would be good for compatibility if, when parsing headers, it ignored incomplete field-lines like other parsers do.

carryel added a commit to carryel/grizzly that referenced this issue Sep 14, 2024
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Oct 24, 2024
carryel added a commit to carryel/grizzly that referenced this issue Nov 18, 2024
+ Added the validation of HTTP header values
+ Name and value's validation are provided as options. (Improved based on #1 @breakponchito)
carryel added a commit to carryel/grizzly that referenced this issue Nov 19, 2024
+ trivial) updated license and re-trigger status checks(eclipse-ee4j#2213).
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Nov 19, 2024
+ Added the validation of HTTP header values
+ Name and value's validation are provided as options. (Improved based on carryel#1 @breakponchito)
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Nov 19, 2024
+ trivial) updated license and re-trigger status checks(eclipse-ee4j#2213).
Pandrex247 pushed a commit to Pandrex247/patched-src-grizzly that referenced this issue Dec 9, 2024
Pandrex247 pushed a commit to Pandrex247/patched-src-grizzly that referenced this issue Dec 9, 2024
+ Added the validation of HTTP header values
+ Name and value's validation are provided as options. (Improved based on carryel#1 @breakponchito)
Pandrex247 pushed a commit to Pandrex247/patched-src-grizzly that referenced this issue Dec 9, 2024
+ trivial) updated license and re-trigger status checks(eclipse-ee4j#2213).
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Dec 11, 2024
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Dec 11, 2024
+ Added the validation of HTTP header values
+ Name and value's validation are provided as options. (Improved based on carryel#1 @breakponchito)
breakponchito pushed a commit to breakponchito/patched-src-grizzly that referenced this issue Dec 11, 2024
+ trivial) updated license and re-trigger status checks(eclipse-ee4j#2213).
carryel added a commit to carryel/grizzly that referenced this issue Dec 14, 2024
+ Field values ​​cannot have a single LF as per RFC-9110(https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5).
+ Additionally, this patch automatically removes support for multiple lines of http headers as mentioned in RFC-2616(https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2).
Note) The features related to this issue eclipse-ee4j#2212 only work when the STRICT_HEADER_NAME_VALIDATION_RFC_9110 and STRICT_HEADER_VALUE_VALIDATION_RFC_9110 options are enabled, and the existing code base behavior is maintained when options are not enabled.

+ Instead of throwing ArrayIndexOutOfBoundsException when judging Token and Text char, it ensures checking within the array range.
+ Since several existing http testcases do not respect CRLF in header values, I modified them to comply with the spec where it was not intentional.

This patch passed all grizzly existing testcases locally, depending on the presence of options.
> mvn clean test
All passed.
> mvn clean test -Dorg.glassfish.grizzly.http.STRICT_HEADER_NAME_VALIDATION_RFC_9110=true -Dorg.glassfish.grizzly.http.STRICT_HEADER_VALUE_VALIDATION_RFC_9110=true
All passed.
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

No branches or pull requests

1 participant