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 8c87d84970fccb1354ea6dbeffaea9e59b55f30b Author: Masakazu Kitajo <[email protected]> AuthorDate: Wed Feb 28 09:41:44 2024 -0700 Fix a bug in parsing chunked messages (#11073) * Fix a bug in parsing chunked messages * Add tests * Fix another case * Allow LF (cherry picked from commit 47e23d7d278a0562bcf27c170a32e5241a04a9e5) --- src/proxy/http/HttpTunnel.cc | 13 ++++-- .../chunked_encoding/bad_chunked_encoding.test.py | 17 +++++--- .../replays/malformed_chunked_header.replay.yaml | 49 ++++++++++++++++++++-- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/src/proxy/http/HttpTunnel.cc b/src/proxy/http/HttpTunnel.cc index f5187f5fc4..16d4eca89d 100644 --- a/src/proxy/http/HttpTunnel.cc +++ b/src/proxy/http/HttpTunnel.cc @@ -157,8 +157,9 @@ ChunkedHandler::read_size() } } else { // We are done parsing size - if (num_digits == 0 || running_sum < 0) { - // Bogus chunk size + if ((num_digits == 0 || running_sum < 0) || /* Bogus chunk size */ + (!ParseRules::is_wslfcr(*tmp) && *tmp != ';') /* Unexpected character */ + ) { state = CHUNK_READ_ERROR; done = true; break; @@ -175,10 +176,16 @@ ChunkedHandler::read_size() break; } } else if (state == CHUNK_READ_SIZE_START) { - if (ParseRules::is_lf(*tmp)) { + if (ParseRules::is_cr(*tmp)) { + // Skip it + } else if (ParseRules::is_lf(*tmp) && + bytes_used <= 2) { // bytes_used should be 2 if it's CRLF, but permit a single LF as well running_sum = 0; num_digits = 0; state = CHUNK_READ_SIZE; + } else { // Unexpected character + state = CHUNK_READ_ERROR; + done = true; } } tmp++; diff --git a/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py b/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py index 84d528c1c6..ccfd8f4a04 100644 --- a/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py +++ b/tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py @@ -125,13 +125,15 @@ class MalformedChunkHeaderTest: def setupOriginServer(self): self.server = Test.MakeVerifierServerProcess("verifier-server2", self.chunkedReplayFile) - # The server's responses will fail the first two transactions + # The server's responses will fail the first three transactions # because ATS will close the connection due to the malformed # chunk headers. self.server.Streams.stdout += Testers.ContainsExpression( "Unexpected chunked content for key 1: too small", "Verify that writing the first response failed.") + self.server.Streams.stdout += Testers.ExcludesExpression( + "chunked body of 3 bytes for key 2 with chunk stream", "Verify that writing the second response failed.") self.server.Streams.stdout += Testers.ContainsExpression( - "Unexpected chunked content for key 2: too small", "Verify that writing the second response failed.") + "Unexpected chunked content for key 3: too small", "Verify that writing the third response failed.") # ATS should close the connection before any body gets through. "abc" # is the body sent by the client for each of these chunked cases. @@ -169,12 +171,15 @@ class MalformedChunkHeaderTest: # The aborted connections will result in errors and a non-zero return # code from the verifier client. tr.Processes.Default.ReturnCode = 1 - tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( - r"(Unexpected chunked content for key 3: too small|Failed HTTP/1 transaction with key: 3)", - "Verify that ATS closed the third transaction.") tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( r"(Unexpected chunked content for key 4: too small|Failed HTTP/1 transaction with key: 4)", - "Verify that ATS closed the fourth transaction.") + "Verify that ATS closed the forth transaction.") + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + r"(Unexpected chunked content for key 5: too small|Failed HTTP/1 transaction with key: 5)", + "Verify that ATS closed the fifth transaction.") + tr.Processes.Default.Streams.stdout += Testers.ContainsExpression( + r"(Unexpected chunked content for key 6: too small|Failed HTTP/1 transaction with key: 6)", + "Verify that ATS closed the sixth transaction.") # ATS should close the connection before any body gets through. "def" # is the body sent by the server for each of these chunked cases. diff --git a/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml b/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml index fd551941e6..ae135d77ab 100644 --- a/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml +++ b/tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml @@ -42,12 +42,32 @@ sessions: - client-request: method: "POST" version: "1.1" - url: /large/chunk/size/header + url: /malformed/chunk/header2 headers: fields: - [ Host, example.com ] - [ Transfer-Encoding, chunked ] - [ uuid, 2 ] + content: + transfer: plain + encoding: uri + # Chunk header sizes are in hex, so a size of `3z` is malformed. + data: 3z%0D%0Aabc%0D%0A0%0D%0A%0D%0A + + # The connection will be dropped and this response will not go out. + server-response: + status: 200 + +- transactions: + - client-request: + method: "POST" + version: "1.1" + url: /large/chunk/size/header + headers: + fields: + - [ Host, example.com ] + - [ Transfer-Encoding, chunked ] + - [ uuid, 3 ] content: transfer: plain encoding: uri @@ -70,7 +90,7 @@ sessions: headers: fields: - [ Host, example.com ] - - [ uuid, 3 ] + - [ uuid, 4 ] # The connection will be dropped and this response will not go out. server-response: @@ -85,6 +105,29 @@ sessions: # Chunk header sizes are in hex, so a size of `z` is malformed. data: z%0D%0Adef%0D%0A0%0D%0A%0D%0A +- transactions: + - client-request: + method: "GET" + version: "1.1" + url: /response/malformed/chunk/size2 + headers: + fields: + - [ Host, example.com ] + - [ uuid, 5 ] + + # The connection will be dropped and this response will not go out. + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Transfer-Encoding, chunked ] + content: + transfer: plain + encoding: uri + # Chunk header sizes are in hex, so a size of `1z` is malformed. + data: 1z%0D%0Adef%0D%0A0%0D%0A%0D%0A + - transactions: - client-request: method: "GET" @@ -93,7 +136,7 @@ sessions: headers: fields: - [ Host, example.com ] - - [ uuid, 4 ] + - [ uuid, 6 ] # The connection will be dropped and this response will not go out. server-response:
