This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new c9206d7315 Fix a bug in parsing chunked messages (#11073)
c9206d7315 is described below
commit c9206d7315c400edbc5a6be3a10e54384266bcf1
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)
---
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/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index dadd6c8ce0..25beb78e4c 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -153,8 +153,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;
@@ -171,10 +172,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 3b163259ce..e92181ccdf 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:
