This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit a4165130eeffd608adbccae8b5e0fc3c3d78902f Author: Brian Neradt <[email protected]> AuthorDate: Thu Jul 11 16:11:50 2024 -0500 Http/2: Fix a crash with empty bodied Expect: 100-continue (#11536) This fixes a crash in processing an empty-bodied HTTP/2 request with an Expect: 100-continue header. Such requests are technically non-RFC compliant, but we should not crash if they happen. With this patch, such requests will be handled as an otherwise compliant Expect: 100-Continue request. That is, the request will be forwarded to the origin, the origin's 100 Continue will be proxied to the client, and then the origin's 200 OK will also be proxied to the client. Fixes #9857 (cherry picked from commit 2cda90d2007a8c2fbc7d56ff7c1fcaae47fb06fa) --- include/proxy/http/HttpSM.h | 7 +++ src/proxy/http/HttpSM.cc | 5 ++ tests/gold_tests/h2/expect_100_continue.yaml | 65 ++++++++++++++++++++++ .../h2/gold/http-request-method-metrics.gold | 2 +- tests/gold_tests/h2/h2origin.test.py | 13 ++++- 5 files changed, 89 insertions(+), 3 deletions(-) diff --git a/include/proxy/http/HttpSM.h b/include/proxy/http/HttpSM.h index d0eda2fea2..893c01ce3d 100644 --- a/include/proxy/http/HttpSM.h +++ b/include/proxy/http/HttpSM.h @@ -436,6 +436,13 @@ private: HttpTunnelProducer *setup_cache_read_transfer(); void setup_internal_transfer(HttpSMHandler handler); void setup_error_transfer(); + + /** Prepare for sending both the 100 Continue and the second response header. + * + * This function sets up the tunnel to send the 100 Continue response and + * then prepares the state machine to send the second response that comes + * after the body is sent. + */ void setup_100_continue_transfer(); HttpTunnelProducer *setup_push_transfer_to_cache(); void setup_transform_to_server_transfer(); diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 265575d13b..2b2119ae76 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -6190,6 +6190,11 @@ close_connection: void HttpSM::do_setup_client_request_body_tunnel(HttpVC_t to_vc_type) { + if (t_state.hdr_info.request_content_length == 0) { + // No tunnel is needed to transfer 0 bytes. Simply return without setting up + // a tunnel nor any of the other related logic around request bodies. + return; + } bool chunked = t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING || t_state.hdr_info.request_content_length == HTTP_UNDEFINED_CL; bool post_redirect = false; diff --git a/tests/gold_tests/h2/expect_100_continue.yaml b/tests/gold_tests/h2/expect_100_continue.yaml new file mode 100644 index 0000000000..7c49460a7a --- /dev/null +++ b/tests/gold_tests/h2/expect_100_continue.yaml @@ -0,0 +1,65 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: '1.0' + +sessions: + - protocol: + - name: http + version: '2' + - name: tls + version: TLSv1.3 + sni: data.brian.example.com + - name: tcp + - name: ip + version: '4' + + transactions: + - client-request: + headers: + encoding: esc_json + fields: + - [ :method, POST ] + - [ :scheme, https ] + - [ :authority, data.brian.example.com ] + - [ :path, /expect/post ] + - [ uuid, 100-continue ] + - [ Expect, 100-continue ] + - [ Content-Length, 0 ] + content: + encoding: plain + size: 0 + + proxy-request: + headers: + encoding: esc_json + fields: + - [ Expect, { value: 100-continue, as: equal } ] + + server-response: + status: 200 + headers: + encoding: esc_json + fields: + - [ Content-Length, 32 ] + - [ X-Response, response_to_post ] + content: + encoding: plain + size: 32 + + proxy-response: + status: 200 diff --git a/tests/gold_tests/h2/gold/http-request-method-metrics.gold b/tests/gold_tests/h2/gold/http-request-method-metrics.gold index 8f930bd6f7..f949dc4270 100644 --- a/tests/gold_tests/h2/gold/http-request-method-metrics.gold +++ b/tests/gold_tests/h2/gold/http-request-method-metrics.gold @@ -1,3 +1,3 @@ proxy.process.http.get_requests 4 -proxy.process.http.post_requests 10 +proxy.process.http.post_requests 11 proxy.process.http.put_requests 0 diff --git a/tests/gold_tests/h2/h2origin.test.py b/tests/gold_tests/h2/h2origin.test.py index 8f5680a200..24e92b553e 100644 --- a/tests/gold_tests/h2/h2origin.test.py +++ b/tests/gold_tests/h2/h2origin.test.py @@ -32,6 +32,7 @@ ts = Test.MakeATSProcess("ts", enable_tls="true") ts.addDefaultSSLFiles() replay_file = "replay_h2origin/" server = Test.MakeVerifierServerProcess("h2-origin", replay_file) +server_expect = Test.MakeVerifierServerProcess("server-expect", "expect_100_continue.yaml") ts.Disk.records_config.update( { 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), @@ -48,7 +49,8 @@ ts.Disk.records_config.update( 'proxy.config.ssl.client.verify.server.policy': 'PERMISSIVE', }) -ts.Disk.remap_config.AddLine('map / https://127.0.0.1:{0}'.format(server.Variables.https_port)) +ts.Disk.remap_config.AddLines( + [f'map /expect http://127.0.0.1:{server_expect.Variables.http_port}', f'map / https://127.0.0.1:{server.Variables.https_port}']) ts.Disk.ssl_multicert_config.AddLine('dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key') ts.Disk.logging_yaml.AddLines( @@ -70,7 +72,14 @@ tr.AddVerifierClientProcess("client", replay_file, http_ports=[ts.Variables.port tr.StillRunningAfter = ts tr.TimeOut = 60 -tr = Test.AddTestRun("Wait squid.log to be written") +# A regression test for #9857. +tr = Test.AddTestRun("Test an empty body POST request with an Expect: 100-continue header") +tr.AddVerifierClientProcess( + "client-expect", "expect_100_continue.yaml", http_ports=[ts.Variables.port], https_ports=[ts.Variables.ssl_port]) +tr.Processes.Default.StartBefore(server_expect) +tr.Processes.Default.ReturnCode = 0 + +tr = Test.AddTestRun("Wait for the squid.log to be written") timeout = 30 watcher = tr.Processes.Process("watcher") watcher.Command = f"sleep {timeout}"
