Skip to content

Commit

Permalink
Improve CRLF skipping for the available method
Browse files Browse the repository at this point in the history
The bytes will now be consumed instead.
Also add processing for the chunk header since those are not actual
data.
Likely related to BZ69545 since the supposedly offending commit was
causing a testsuite failure following the TestChunkedInputFilter update.
  • Loading branch information
rmaucher committed Jan 30, 2025
1 parent 0820667 commit 73c067b
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 3 deletions.
121 changes: 118 additions & 3 deletions java/org/apache/coyote/http11/filters/ChunkedInputFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,32 @@ public int available() {
available = readChunk.remaining();
}

// Handle some edge cases
if (available > 2 && (parseState == ParseState.CHUNK_BODY_CRLF || parseState == ParseState.CHUNK_HEADER)) {
if (parseState == ParseState.CHUNK_BODY_CRLF) {
skipCRLF();
}
if (parseState == ParseState.CHUNK_HEADER) {
skipChunkHeader();
}
available = readChunk.remaining();
// If ending as TRAILER_FIELDS, then the next read will be EOF and available can be > 0
// If ending as CHUNK_HEADER then there's nothing left to read for now
// If ending as CHUNK_BODY_CRLF, then if there's more than two left, there will be data to read or leading to EOF
}
if (available == 1 && parseState == ParseState.CHUNK_BODY_CRLF) {
// Either just the CR or just the LF are left in the buffer. There is no data to read.
available = 0;
if (!skipCRLF()) {
available = readChunk.remaining();
} else {
available = 0;
}
} else if (available == 2 && !crFound && parseState == ParseState.CHUNK_BODY_CRLF) {
// Just CRLF is left in the buffer. There is no data to read.
available = 0;
if (!skipCRLF()) {
available = readChunk.remaining();
} else {
available = 0;
}
}

if (available == 0) {
Expand Down Expand Up @@ -391,6 +410,69 @@ private boolean parseChunkHeader() throws IOException {
}


private boolean skipChunkHeader() {

boolean eol = false;

while (!eol) {
if (readChunk == null || readChunk.position() >= readChunk.limit()) {
return false;
}

byte chr = readChunk.get(readChunk.position());
if (chr == Constants.CR || chr == Constants.LF) {
parsingExtension = false;
if (!skipCRLF()) {
return false;
}
eol = true;
} else if (chr == Constants.SEMI_COLON && !parsingExtension) {
// First semi-colon marks the start of the extension. Further
// semi-colons may appear to separate multiple chunk-extensions.
// These need to be processed as part of parsing the extensions.
parsingExtension = true;
extensionSize++;
} else if (!parsingExtension) {
int charValue = HexUtils.getDec(chr);
if (charValue != -1 && chunkSizeDigitsRead < 8) {
chunkSizeDigitsRead++;
remaining = (remaining << 4) | charValue;
} else {
// Isn't valid hex so this is an error condition
return false;
}
} else {
// Extension 'parsing'
// Note that the chunk-extension is neither parsed nor
// validated. Currently it is simply ignored.
extensionSize++;
if (maxExtensionSize > -1 && extensionSize > maxExtensionSize) {
return false;
}
}

// Parsing the CRLF increments pos
if (!eol) {
readChunk.position(readChunk.position() + 1);
}
}

if (chunkSizeDigitsRead == 0 || remaining < 0) {
return false;
} else {
chunkSizeDigitsRead = 0;
}

if (remaining == 0) {
parseState = ParseState.TRAILER_FIELDS;
} else {
parseState = ParseState.CHUNK_BODY;
}

return true;
}


private int parseChunkBody(ApplicationBufferHandler handler) throws IOException {
int result = 0;

Expand Down Expand Up @@ -461,6 +543,39 @@ private boolean parseCRLF() throws IOException {
}


private boolean skipCRLF() {

boolean eol = false;

while (!eol) {
if (readChunk == null || readChunk.position() >= readChunk.limit()) {
return false;
}

byte chr = readChunk.get(readChunk.position());
if (chr == Constants.CR) {
if (crFound) {
return false;
}
crFound = true;
} else if (chr == Constants.LF) {
if (!crFound) {
return false;
}
eol = true;
} else {
return false;
}

readChunk.position(readChunk.position() + 1);
}

crFound = false;
parseState = ParseState.CHUNK_HEADER;
return true;
}


/**
* Parse end chunk data.
*
Expand Down
4 changes: 4 additions & 0 deletions webapps/docs/changelog.xml
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@
<strong>allowBackslash</strong> attributes determines how the decoded
URI is processed. (markt)
</add>
<fix>
<bug>69545</bug>: Improve CRLF skipping for the <code>available</code>
method of the <code>ChunkedInputFilter</code>. (remm)
</fix>
</changelog>
</subsection>
<subsection name="Jasper">
Expand Down

0 comments on commit 73c067b

Please sign in to comment.