Validate all TE header instances for HTTP/2 request conformance.#620
Validate all TE header instances for HTTP/2 request conformance.#620arturobernalg wants to merge 1 commit intoapache:masterfrom
Conversation
| final String value = header.getValue(); | ||
| if (!"trailers".equalsIgnoreCase(value)) { | ||
|
|
||
| for (final String headerName : illegalHeaderNames) { |
There was a problem hiding this comment.
@arturobernalg enhanced for loop creates an extra iterator, which is unnecessary Please keep the simple loop.
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java
Show resolved
Hide resolved
arturobernalg
force-pushed
the
te
branch
2 times, most recently
from
1699f8f to
1342a90
Compare
httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestConformance.java
Outdated
Show resolved
Hide resolved
| "Header '%s: %s' is illegal for HTTP/2 messages", HttpHeaders.TE, token); | ||
| } | ||
| } | ||
| if (!sawAnyToken && request.getFirstHeader(HttpHeaders.TE) != null) { |
There was a problem hiding this comment.
@arturobernalg This is bad. Why would you want to do a look-up on here, again? You do you need this line at all? If there has been no ProtocolException no exception at this point, everything is OK.
There was a problem hiding this comment.
You need some equivalent of sawAnyToken if you want to reject these cases:
- TE:
- TE:
- TE: , , (only separators / whitespace)
There was a problem hiding this comment.
@arturobernalg I do not see a problem with ignoring TE: or TE: . 'TE: , , , ,' however will get correctly rejected.
If you want to eliminate the former two cases as well, I think there is a way of doing without resorting to an extra #getFirstHeader call.
Reject any TE value other than trailers (including additional TE headers).
2 participants
Reject any TE value other than trailers (including additional TE headers).