Skip to content

Commit 234bb4a

Browse files
authored
Fix prev_is_cr flag handling in chunked encoding parser (#13036)
The prev_is_cr flag in ChunkedHandler::read_size() was being set within conditional expressions but not consistently updated, which could cause it to retain stale values across parsing iterations. Update the flag at the end of each loop iteration to ensure it accurately reflects the current character state. This improves the correctness of line break validation when parsing chunked transfer encoding with strict_chunk_parsing enabled.
1 parent 2baa396 commit 234bb4a

3 files changed

Lines changed: 31 additions & 3 deletions

File tree

src/proxy/http/HttpTunnel.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ ChunkedHandler::read_size()
191191
done = true;
192192
break;
193193
} else {
194-
if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
194+
if (ParseRules::is_cr(*tmp)) {
195195
++num_cr;
196196
}
197197
state = ChunkedState::READ_SIZE_CRLF; // now look for CRLF
@@ -213,7 +213,7 @@ ChunkedHandler::read_size()
213213
done = true;
214214
num_cr = 0;
215215
break;
216-
} else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
216+
} else if (ParseRules::is_cr(*tmp)) {
217217
if (num_cr != 0) {
218218
state = ChunkedState::READ_ERROR;
219219
done = true;
@@ -236,7 +236,7 @@ ChunkedHandler::read_size()
236236
num_digits = 0;
237237
num_cr = 0;
238238
state = ChunkedState::READ_SIZE;
239-
} else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
239+
} else if (ParseRules::is_cr(*tmp)) {
240240
if (num_cr != 0) {
241241
Dbg(dbg_ctl_http_chunk, "Found multiple CRs before chunk size");
242242
state = ChunkedState::READ_ERROR;
@@ -249,9 +249,15 @@ ChunkedHandler::read_size()
249249
done = true;
250250
}
251251
}
252+
prev_is_cr = ParseRules::is_cr(*tmp);
252253
tmp++;
253254
data_size--;
254255
}
256+
257+
if (data_size > 0) {
258+
prev_is_cr = ParseRules::is_cr(*tmp);
259+
}
260+
255261
if (drop_chunked_trailers) {
256262
chunked_buffer->write(chunked_reader, bytes_used);
257263
chunked_size += bytes_used;

tests/gold_tests/chunked_encoding/bad_chunked_encoding.test.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ def setupOriginServer(self):
140140
"chunked body of 3 bytes for key 2 with chunk stream", "Verify that writing the second response failed.")
141141
self.server.Streams.stdout += Testers.ContainsExpression(
142142
"Unexpected chunked content for key 3: too small", "Verify that writing the third response failed.")
143+
self.server.Streams.stdout += Testers.ContainsExpression(
144+
"Unexpected chunked content for key 8: too small", "Verify that writing the sixth response failed.")
143145

144146
# ATS should close the connection before any body gets through. "abcwxyz"
145147
# is the body sent by the client for each of these chunked cases.

tests/gold_tests/chunked_encoding/replays/malformed_chunked_header.replay.yaml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,26 @@ sessions:
158158
server-response:
159159
status: 200
160160

161+
- transactions:
162+
- client-request:
163+
method: "POST"
164+
version: "1.1"
165+
url: /malformed/chunk/header3
166+
headers:
167+
fields:
168+
- [ Host, example.com ]
169+
- [ Transfer-Encoding, chunked ]
170+
- [ uuid, 8 ]
171+
content:
172+
transfer: plain
173+
encoding: uri
174+
# chunk-size is set to 1, but no chunk-data is present.
175+
data: 1%0D%0A%0D%0A0%0D%0A%0D%0A
176+
177+
# The connection will be dropped and this response will not go out.
178+
server-response:
179+
status: 200
180+
161181
#
162182
# Now repeat the above two malformed chunk header tests, but on the server
163183
# side.

0 commit comments

Comments
 (0)