Skip to content

Commit

Permalink
Fixes #9141 - Thread-safe Content.Chunk#slice
Browse files Browse the repository at this point in the history
* Changed Content.Chunk.slice(int, int, boolean) to have the same parameters as ByteBuffer.slice(int, int) for consistency.
* Updated Chunk.slice(int, int, boolean) javadocs.
* Update code that was calling Chunk.slice(int, int, boolean).

Signed-off-by: Simone Bordet <[email protected]>
  • Loading branch information
sbordet committed Jan 9, 2023
1 parent f14c0a5 commit 16065fc
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1203,11 +1203,14 @@ private boolean parseContent(Content.Chunk chunk)
int boundaryOffset = boundaryFinder.match(buffer);
if (boundaryOffset >= 0)
{
int sliceLimit = buffer.position() + boundaryOffset;
if (sliceLimit > 0 && buffer.get(sliceLimit - 1) == '\r')
--sliceLimit;
Content.Chunk content = chunk.slice(buffer.position(), sliceLimit, true);
buffer.position(buffer.position() + boundaryOffset + boundaryFinder.getLength());
int position = buffer.position();
int length = boundaryOffset;
// BoundaryFinder is configured to search for '\n--Boundary';
// if we found '\r\n--Boundary' then the '\r' is not content.
if (length > 0 && buffer.get(position + length - 1) == '\r')
--length;
Content.Chunk content = chunk.slice(position, length, true);
buffer.position(position + boundaryOffset + boundaryFinder.getLength());
notifyPartContent(content);
notifyPartEnd();
return true;
Expand All @@ -1217,15 +1220,19 @@ private boolean parseContent(Content.Chunk chunk)
partialBoundaryMatch = boundaryFinder.endsWith(buffer);
if (partialBoundaryMatch > 0)
{
int sliceLimit = buffer.limit() - partialBoundaryMatch;
int limit = buffer.limit();
int sliceLimit = limit - partialBoundaryMatch;
// BoundaryFinder is configured to search for '\n--Boundary';
// if we found '\r\n--Bo' then the '\r' may not be content,
// but remember it in case there is a boundary mismatch.
if (sliceLimit > 0 && buffer.get(sliceLimit - 1) == '\r')
{
// Remember that there was a CR in case there will be a boundary mismatch.
crContent = true;
--sliceLimit;
}
Content.Chunk content = chunk.slice(buffer.position(), sliceLimit, false);
buffer.position(buffer.limit());
int position = buffer.position();
Content.Chunk content = chunk.slice(position, sliceLimit - position, false);
buffer.position(limit);
if (content.hasRemaining())
notifyPartContent(content);
return false;
Expand All @@ -1237,13 +1244,17 @@ private boolean parseContent(Content.Chunk chunk)
crContent = false;
notifyPartContent(Content.Chunk.from(CR.slice(), false));
}
// If '\r' is found at the end of the buffer, it may
// not be content but the beginning of a '\r\n--Boundary';
// remember it in case it is truly normal content.
int sliceLimit = buffer.limit();
if (buffer.get(sliceLimit - 1) == '\r')
{
crContent = true;
--sliceLimit;
}
Content.Chunk content = chunk.slice(buffer.position(), sliceLimit, false);
int position = buffer.position();
Content.Chunk content = chunk.slice(position, sliceLimit - position, false);
buffer.position(buffer.limit());
if (content.hasRemaining())
notifyPartContent(content);
Expand Down
32 changes: 15 additions & 17 deletions jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java
Original file line number Diff line number Diff line change
Expand Up @@ -596,36 +596,34 @@ default Chunk slice()
}

/**
* <p>Returns a new {@code Chunk} whose {@code ByteBuffer} is a slice, with the given
* position and limit, of the {@code ByteBuffer} of the source {@code Chunk} unless the
* source is {@link #isTerminal() terminal} in which case {@code this} is returned, or
* if {@code position == limit} in which case {@link #EOF} or {@link #EMPTY} is
* returned depending on the value of {@code last}.</p>
* <p>Returns a new {@code Chunk} whose {@code ByteBuffer} is a slice of the
* {@code ByteBuffer} of the this {@code Chunk}.</p>
* <p>The returned {@code Chunk} is:</p>
* <ul>
* <li>{@code this}, if this {@code Chunk} is {@link #isTerminal() terminal}</li>
* <li>{@link #EMPTY}, if {@code length == 0 && !last}</li>
* <li>{@link #EOF}, if {@code length == 0 && last}</li>
* <li>a new {@code Chunk} whose {@code ByteBuffer} is a slice of the
* {@code ByteBuffer} of this {@code Chunk}, from the given {@code position}
* and for the given {@code length} bytes.</li>
* </ul>
* <p>The returned {@code Chunk} retains the source {@code Chunk} and it is linked
* to it via {@link #from(ByteBuffer, boolean, Retainable)}.</p>
*
* @param position the position at which the slice begins
* @param limit the limit at which the slice ends
* @param length the length of the slice
* @param last whether the new Chunk is last
* @return a new {@code Chunk} retained from the source {@code Chunk} with a slice
* of the source {@code Chunk}'s {@code ByteBuffer}
*/
default Chunk slice(int position, int limit, boolean last)
default Chunk slice(int position, int length, boolean last)
{
if (isTerminal())
return this;
if (position == limit)
if (length == 0)
return last ? EOF : EMPTY;
ByteBuffer sourceBuffer = getByteBuffer();
int sourceLimit = sourceBuffer.limit();
sourceBuffer.limit(limit);
int sourcePosition = sourceBuffer.position();
sourceBuffer.position(position);
ByteBuffer slice = sourceBuffer.slice();
sourceBuffer.limit(sourceLimit);
sourceBuffer.position(sourcePosition);
retain();
return from(slice, last, this);
return from(getByteBuffer().slice(position, length), last, this);
}

/**
Expand Down

0 comments on commit 16065fc

Please sign in to comment.