zwoop commented on code in PR #13420:
URL: https://github.com/apache/trafficserver/pull/13420#discussion_r4137599886
##########
src/proxy/http2/Http2Stream.cc:
##########
@@ -327,6 +372,73 @@ Http2Stream::send_headers(Http2ConnectionState & /* cstate
ATS_UNUSED */)
this->_http_sm_id = this->_sm->sm_id;
}
+ // parse_req is skipped here; re-apply strict_uri_parsing. Runs after the
REQUEST type
+ // check below, since path_get() asserts that polarity.
+ 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 token methods, Host and Content-Length framing
(RFC 9110 8.6),
+ // and the request line and header field size limits.
+ auto parse_req_would_accept = [&]() {
+ auto const *config = this->_sm->t_state.http_config_param;
+ auto const line_max =
static_cast<size_t>(config->http_request_line_max_size);
+ auto const field_max =
static_cast<size_t>(config->http_hdr_field_max_size);
+ 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;
+ }
+ // Upper bound of the serialized "METHOD URL HTTP/1.1\r\n", so parse_req
makes the exact call.
+ if (method.size() +
static_cast<size_t>(_receive_header.url_get()->length_get()) + 12 > line_max) {
+ return false;
+ }
+ for (auto const &field : _receive_header) {
+ auto name{field.name_get()};
+ auto value{field.value_get()};
+
+ // parse_req drops a line whose name starts with a non-token and trims
whitespace around values.
+ if (name.empty() || name.size() + value.size() > field_max ||
(!ParseRules::is_token(name.front()) && name.front() != '@') ||
+ (!value.empty() && (ParseRules::is_ws(value.front()) ||
ParseRules::is_ws(value.back())))) {
+ return false;
+ }
+ }
+ // url_parse_internet() accepts the userinfo that RFC 9113 8.3.1 bans from
:authority,
+ // and the Host built from it never sees validate_hdr_host(). Mirror that
check here.
+ if (MIMEField *host =
_receive_header.field_find(static_cast<std::string_view>(MIME_FIELD_HOST));
host != nullptr) {
+ std::string_view parsed_host;
+ int port = 0;
+ bool has_port = false;
+
+ if (host->has_dups() || !http_parse_host_header(host->value_get(),
parsed_host, port, has_port)) {
+ return false;
+ }
+ }
+ return validate_hdr_content_length(_receive_header.m_heap,
_receive_header.m_http) == ParseResult::DONE;
+ };
+
+ // A failed conversion leaves a \xffVOID method that only parse_req can turn
into a 400.
+ if (conversion_ok && !this->trailing_header_is_possible() &&
!this->is_outbound_connection() &&
+ _receive_header.type_get() == HTTPType::REQUEST && this->_sm != nullptr
&& this->read_vio.nbytes > 0 && uri_ok() &&
+ parse_req_would_accept()) {
+ // The stream owns _receive_header and outlives the handoff, so the pulled
pointer cannot dangle.
+ this->_is_parsed_receive_header_ready = true;
Review Comment:
Fixed by taking the fast path only when HEADERS carries END_STREAM, so no
trailer can follow and reset _receive_header before HttpSM copies it. Requests
with a body use the serialize path.
##########
src/proxy/http2/Http2Stream.cc:
##########
@@ -850,15 +964,33 @@ Http2Stream::update_write_request(bool call_update)
// Process the new data
if (!this->parsing_header_done) {
- // Still parsing the request or response header
int bytes_used = 0;
ParseResult state;
- if (this->is_outbound_connection()) {
+ HTTPHdr *send_hdr = this->_pending_send_header();
+
+ if (send_hdr != nullptr) {
Review Comment:
Not changing this. Hook-set headers come from admin-controlled plugins. A
conflicting Content-Length or invalid Host from a hook also failed on the
previous path, and HTTP/1.1 forwards hook-set headers without validating them.
For HTTP/2 the DATA frames define framing.
--
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]
