Skip to content

Commit e5accd7

Browse files
authored
Fix prev_is_cr flag handling in chunked encoding parser (#13038)
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 b789398 commit e5accd7

3 files changed

Lines changed: 31 additions & 3 deletions

File tree

proxy/http/HttpTunnel.cc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ ChunkedHandler::read_size()
179179
done = true;
180180
break;
181181
} else {
182-
if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
182+
if (ParseRules::is_cr(*tmp)) {
183183
++num_cr;
184184
}
185185
state = CHUNK_READ_SIZE_CRLF; // now look for CRLF
@@ -201,7 +201,7 @@ ChunkedHandler::read_size()
201201
done = true;
202202
num_cr = 0;
203203
break;
204-
} else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
204+
} else if (ParseRules::is_cr(*tmp)) {
205205
if (num_cr != 0) {
206206
state = CHUNK_READ_ERROR;
207207
done = true;
@@ -224,7 +224,7 @@ ChunkedHandler::read_size()
224224
num_digits = 0;
225225
num_cr = 0;
226226
state = CHUNK_READ_SIZE;
227-
} else if ((prev_is_cr = ParseRules::is_cr(*tmp)) == true) {
227+
} else if (ParseRules::is_cr(*tmp)) {
228228
if (num_cr != 0) {
229229
Debug("http_chunk", "Found multiple CRs before chunk size");
230230
state = CHUNK_READ_ERROR;
@@ -237,9 +237,15 @@ ChunkedHandler::read_size()
237237
done = true;
238238
}
239239
}
240+
prev_is_cr = ParseRules::is_cr(*tmp);
240241
tmp++;
241242
data_size--;
242243
}
244+
245+
if (data_size > 0) {
246+
prev_is_cr = ParseRules::is_cr(*tmp);
247+
}
248+
243249
if (drop_chunked_trailers) {
244250
chunked_buffer->write(chunked_reader, bytes_used);
245251
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
@@ -134,6 +134,8 @@ def setupOriginServer(self):
134134
"chunked body of 3 bytes for key 2 with chunk stream", "Verify that writing the second response failed.")
135135
self.server.Streams.stdout += Testers.ContainsExpression(
136136
"Unexpected chunked content for key 3: too small", "Verify that writing the third response failed.")
137+
self.server.Streams.stdout += Testers.ContainsExpression(
138+
"Unexpected chunked content for key 8: too small", "Verify that writing the sixth response failed.")
137139

138140
# ATS should close the connection before any body gets through. "abcwxyz"
139141
# 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
@@ -119,6 +119,26 @@ sessions:
119119
status: 200
120120

121121

122+
- transactions:
123+
- client-request:
124+
method: "POST"
125+
version: "1.1"
126+
url: /malformed/chunk/header3
127+
headers:
128+
fields:
129+
- [ Host, example.com ]
130+
- [ Transfer-Encoding, chunked ]
131+
- [ uuid, 8 ]
132+
content:
133+
transfer: plain
134+
encoding: uri
135+
# chunk-size is set to 1, but no chunk-data is present.
136+
data: 1%0D%0A%0D%0A0%0D%0A%0D%0A
137+
138+
# The connection will be dropped and this response will not go out.
139+
server-response:
140+
status: 200
141+
122142
#
123143
# Now repeat the above two malformed chunk header tests, but on the server
124144
# side.

0 commit comments

Comments
 (0)