Copilot commented on code in PR #13420:
URL: https://github.com/apache/trafficserver/pull/13420#discussion_r3635219921
##########
src/proxy/http/HttpSM.cc:
##########
@@ -627,10 +628,24 @@ HttpSM::state_read_client_request_header(int event, void
*data)
// tokenize header //
/////////////////////
- ParseResult state = t_state.hdr_info.client_request.parse_req(&http_parser,
_ua.get_txn()->get_remote_reader(), &bytes_used,
-
_ua.get_entry()->eos, t_state.http_config_param->strict_uri_parsing,
-
t_state.http_config_param->http_request_line_max_size,
-
t_state.http_config_param->http_hdr_field_max_size);
+ ParseResult state;
+
+ if (_pre_parsed_ua_request != nullptr) {
+ // The UA_FIRST_READ gate above never fires on this path: the read buffer
stays empty.
+ if (milestones[TS_MILESTONE_UA_FIRST_READ] == 0) {
+ ATS_PROBE1(milestone_ua_first_read, sm_id);
+ milestones[TS_MILESTONE_UA_FIRST_READ] = ink_get_hrtime();
+ }
+ t_state.hdr_info.client_request.copy(_pre_parsed_ua_request);
+ _pre_parsed_ua_request = nullptr;
+ bytes_used = t_state.hdr_info.client_request.length_get();
+ state = ParseResult::DONE;
+ } else {
+ state = t_state.hdr_info.client_request.parse_req(&http_parser,
_ua.get_txn()->get_remote_reader(), &bytes_used,
+ _ua.get_entry()->eos,
t_state.http_config_param->strict_uri_parsing,
+
t_state.http_config_param->http_request_line_max_size,
+
t_state.http_config_param->http_hdr_field_max_size);
+ }
Review Comment:
Fast-path pre-parsed request copy bypasses the validations that
http_parser_parse_req() normally applies (method token character check via
ParseRules::is_token, plus
validate_hdr_request_target/validate_hdr_host/validate_hdr_content_length).
This changes behavior relative to the serialize+reparse path and can allow
malformed requests (e.g., non-token :method bytes or invalid/mismatched
Content-Length) to be accepted on the HTTP/2 fast path.
##########
src/proxy/http2/Http2Stream.cc:
##########
@@ -326,6 +335,37 @@ 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()));
+ };
+
+ // The conversion_ok gate keeps a \xffVOID method on the serialize path,
where parse_req 400s it.
+ 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()) {
+ this->_sm->set_pre_parsed_ua_request(&_receive_header);
+ if (this->receive_end_stream) {
+ this->read_vio.nbytes = this->data_length;
+ this->read_vio.ndone = this->read_vio.nbytes;
+ this->signal_read_event(VC_EVENT_READ_COMPLETE);
+ } else {
+ this->has_body = true;
+ this->signal_read_event(VC_EVENT_READ_READY);
+ }
+ // Delivery is synchronous (stream mutex): the borrow is copied, or the txn
+ // finished and _sm is null. A still-pending borrow is unreachable;
recover anyway.
+ if (this->_sm == nullptr ||
!this->_sm->has_pending_pre_parsed_ua_request()) {
+ return;
+ }
+ ink_assert(!"pre-parsed handoff was deferred");
+ this->_sm->set_pre_parsed_ua_request(nullptr);
+ }
Review Comment:
The fast-path assumes synchronous delivery of the pre-parsed header, but
Http2Stream::signal_read_event() uses MUTEX_TRY_LOCK and can defer delivery via
_read_vio_event. In that case this ink_assert("pre-parsed handoff was
deferred") is reachable under normal mutex contention and can crash debug
builds. Treat deferral as a normal fallback (clear the borrow and continue with
serialize+reparse) without asserting.
--
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]
