From 16065fcef7a04c528ddddbbb0301bf2f32f4c8a8 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 9 Jan 2023 17:06:03 +0100 Subject: [PATCH] Fixes #9141 - Thread-safe Content.Chunk#slice * 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 --- .../org/eclipse/jetty/http/MultiPart.java | 31 ++++++++++++------ .../java/org/eclipse/jetty/io/Content.java | 32 +++++++++---------- 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 77e842ddc955..358ac4e329a8 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -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; @@ -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; @@ -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); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 0b0cc93d1121..76bf625907a2 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -596,36 +596,34 @@ default Chunk slice() } /** - *

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}.

+ *

Returns a new {@code Chunk} whose {@code ByteBuffer} is a slice of the + * {@code ByteBuffer} of the this {@code Chunk}.

+ *

The returned {@code Chunk} is:

+ * *

The returned {@code Chunk} retains the source {@code Chunk} and it is linked * to it via {@link #from(ByteBuffer, boolean, Retainable)}.

* * @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); } /**