Skip to content

Commit a78aedc

Browse files
committed
Fix issues found by Copilot reviewing the chunked extension validation
Name only extensions weren't handled Non-blocking reads that ended after CR and before LF then failed Exceptions were inconsistent
1 parent 2a07796 commit a78aedc

File tree

4 files changed

+22
-4
lines changed

4 files changed

+22
-4
lines changed

java/org/apache/coyote/http11/filters/ChunkedInputFilter.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,17 @@ private boolean parseChunkHeader() throws IOException {
359359
byte chr = readChunk.get(readChunk.position());
360360

361361
if (extensionState != null) {
362-
extensionState = ChunkExtension.parse(chr, extensionState);
362+
try {
363+
extensionState = ChunkExtension.parse(chr, extensionState);
364+
} catch (IOException ioe) {
365+
throwBadRequestException(sm.getString("chunkedInputFilter.invalidHeader"));
366+
}
363367
if (extensionState == State.CR) {
368+
extensionState = null;
364369
if (!parseCRLF()) {
365370
return false;
366371
}
367372
eol = true;
368-
extensionState = null;
369373
} else {
370374
// Check the size
371375
long extSize = extensionSize.incrementAndGet();
@@ -444,11 +448,11 @@ private boolean skipChunkHeader() {
444448
return false;
445449
}
446450
if (extensionState == State.CR) {
451+
extensionState = null;
447452
if (!skipCRLF()) {
448453
return false;
449454
}
450455
eol = true;
451-
extensionState = null;
452456
} else {
453457
// Check the size
454458
long extSize = extensionSize.incrementAndGet();

java/org/apache/tomcat/util/http/parser/ChunkExtension.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ public static State parse(byte b, State state) throws IOException {
4545
return State.POST_NAME;
4646
} else if (HttpParser.isToken(c)) {
4747
return State.NAME;
48+
} else if (c == ';') {
49+
return State.PRE_NAME;
4850
} else if (c == '=') {
4951
return State.EQUALS;
5052
} else if (c == '\r') {
@@ -54,6 +56,8 @@ public static State parse(byte b, State state) throws IOException {
5456
case POST_NAME:
5557
if (HttpParser.isWhiteSpace(c)) {
5658
return State.POST_NAME;
59+
} else if (c == ';') {
60+
return State.PRE_NAME;
5761
} else if (c == '=') {
5862
return State.EQUALS;
5963
} else if (c == '\r') {

test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ public void testNonBlockingReadChunkedSplitInFinalCrlf() throws Exception {
599599
public void testNonBlockingReadChunkedSplitMaximum() throws Exception {
600600
// @formatter:off
601601
String requestBody = new String(
602-
"14" + CRLF +
602+
"14;a=b;c" + CRLF +
603603
"012345678901FINISHED" + CRLF +
604604
"0" + CRLF +
605605
TRAILER_HEADER_NAME + ": " + TRAILER_HEADER_VALUE + CRLF +

test/org/apache/tomcat/util/http/parser/TestChunkExtension.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ public void testTokenOnly03() {
7676
doTest("; abc \r\n", true);
7777
}
7878

79+
@Test
80+
public void testTokenOnlyTokenOnly01() {
81+
doTest(";abc;abc\r\n", true);
82+
}
83+
84+
@Test
85+
public void testTokenOnlyTokenOnly02() {
86+
doTest("; abc ; abc \r\n", true);
87+
}
88+
7989
@Test
8090
public void testTokenToken01() {
8191
doTest(";abc=abc\r\n", true);

0 commit comments

Comments
 (0)