Copilot commented on code in PR #13420:
URL: https://github.com/apache/trafficserver/pull/13420#discussion_r3652966153
##########
src/proxy/http2/Http2Stream.cc:
##########
@@ -326,6 +365,50 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate
ATS_UNUSED */)
this->_http_sm_id = this->_sm->sm_id;
}
+ // The fast path skips parse_req, so re-apply strict_uri_parsing on the
target.
+ // Evaluated last so path_get() (asserts REQUEST polarity) only sees a
REQUEST.
+ auto uri_ok = [&]() {
+ int const level = this->_sm->t_state.http_config_param->strict_uri_parsing;
+
+ return level == 0 ||
+ (url_is_uri_compliant(level, _receive_header.path_get()) &&
url_is_uri_compliant(level, _receive_header.query_get()) &&
+ url_is_uri_compliant(level, _receive_header.fragment_get()));
+ };
+
+ // parse_req also enforces a token method and Content-Length framing (RFC
9110 8.6),
+ // which the fast path would otherwise skip.
+ auto parse_req_would_accept = [&]() {
+ auto method{_receive_header.method_get()};
+ if (method.empty() || std::any_of(method.begin(), method.end(), [](char c)
{ return !ParseRules::is_token(c); })) {
+ return false;
+ }
+ if (MIMEField *cl =
_receive_header.field_find(static_cast<std::string_view>(MIME_FIELD_CONTENT_LENGTH));
cl != nullptr) {
+ auto value{cl->value_get()};
+ if (cl->has_dups() || value.empty() || std::any_of(value.begin(),
value.end(), [](char c) { return c < '0' || c > '9'; }) ||
+
_receive_header.field_find(static_cast<std::string_view>(MIME_FIELD_TRANSFER_ENCODING))
!= nullptr) {
+ return false;
+ }
+ }
+ return true;
+ };
Review Comment:
The HTTP/2 fast path currently bypasses `parse_req`/`validate_hdr_host`, but
the fast-path gate (`parse_req_would_accept`) doesn’t validate the synthesized
`Host` header at all. Because `:authority` is parsed with
`url_parse_internet()` (which allows RFC3986 userinfo), a request like
`:authority: user@host` can pass the fast-path checks even though
`validate_host_name()` (used by `validate_hdr_host`) would reject the literal
'@' in `Host` and the legacy path would 400 it. Add a Host-header parity check
(at least reject duplicates/empty and any '@' in the Host value) so such
requests fall back to the serialize+parse path and keep behavior consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
