diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2ServerTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2ServerTest.java index eccbe9c9b4d2..3464edf56a0e 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2ServerTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2ServerTest.java @@ -31,9 +31,11 @@ import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.Flags; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.api.server.ServerSessionListener; import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.PingFrame; @@ -521,20 +523,18 @@ public void testRequestWithContinuationFramesWithEmptyContinuationFrame() throws generator.control(accumulator, new PrefaceFrame()); generator.control(accumulator, new SettingsFrame(new HashMap<>(), false)); MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY); + + long offset = accumulator.size(); generator.control(accumulator, new HeadersFrame(1, metaData, null, true)); - // Take the ContinuationFrame header, duplicate it, and set the length to zero. - /* TODO - List buffers = accumulator.getByteBuffers(); - ByteBuffer continuationFrameHeader = buffers.get(4); - ByteBuffer duplicate = ByteBuffer.allocate(continuationFrameHeader.remaining()); - duplicate.put(continuationFrameHeader).flip(); - continuationFrameHeader.flip(); - continuationFrameHeader.put(0, (byte)0); - continuationFrameHeader.putShort(1, (short)0); - // Insert a CONTINUATION frame header for the body of the previous CONTINUATION frame. - accumulator.add(RetainableByteBuffer.wrap(duplicate)); - */ + RetainableByteBuffer headers = accumulator.take(offset); + RetainableByteBuffer.Mutable continuation = headers.copy().asMutable(); + accumulator.add(headers); + continuation.limit(9); + continuation.put(0, (byte)0x00); + continuation.put(1, (byte)0x00); + continuation.put(2, (byte)0x00); + accumulator.add(continuation); return accumulator; }); } @@ -548,22 +548,40 @@ public void testRequestWithContinuationFramesWithEmptyLastContinuationFrame() th generator.control(accumulator, new PrefaceFrame()); generator.control(accumulator, new SettingsFrame(new HashMap<>(), false)); MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY); + + long offset = accumulator.size(); generator.control(accumulator, new HeadersFrame(1, metaData, null, true)); - /* TODO + + RetainableByteBuffer headers = accumulator.take(offset); + System.err.println(accumulator.toDetailString()); + System.err.println(headers.toDetailString()); + // Take the last CONTINUATION frame and reset the flag. - List buffers = accumulator.getByteBuffers(); - ByteBuffer continuationFrameHeader = buffers.get(buffers.size() - 2); - continuationFrameHeader.put(4, (byte)0); + offset = 0; + while (true) + { + int length = ((headers.get(offset) & 0xFF) << 16) + ((headers.get(offset + 1) & 0xFF) << 8) + (headers.get(offset + 2) & 0xFF); + byte flag = headers.get(offset + 4); + if (flag == 0x04) + { + RetainableByteBuffer.Mutable last = headers.take(offset).asMutable(); + accumulator.add(headers); + last.put(4, (byte)0); + accumulator.add(last); + break; + } + offset += 9 + length; + } + // Add a last, empty, CONTINUATION frame. - ByteBuffer last = ByteBuffer.wrap(new byte[]{ - 0, 0, 0, // Length - (byte)FrameType.CONTINUATION.getType(), - (byte)Flags.END_HEADERS, - 0, 0, 0, 1 // Stream ID - }); - accumulator.append(RetainableByteBuffer.wrap(last)); + accumulator.add( + ByteBuffer.wrap(new byte[]{ + 0, 0, 0, // Length + (byte)FrameType.CONTINUATION.getType(), + (byte)Flags.END_HEADERS, + 0, 0, 0, 1 // Stream ID + })); - */ return accumulator; }); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java index 6182d88681a1..5ebc5c78de7b 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; import java.util.Objects; import org.eclipse.jetty.util.Blocker; @@ -144,7 +145,7 @@ default Mutable asMutable() throws ReadOnlyBufferException throw new ReadOnlyBufferException(); if (this instanceof Mutable mutable) return mutable; - return new FixedCapacity(getByteBuffer(), this); + throw new ReadOnlyBufferException(); } /** @@ -377,7 +378,8 @@ default RetainableByteBuffer slice(long length) /** * Take the contents of this buffer from an index. - * @return A buffer with the contents of this buffer from the index, avoiding copies if possible. + * @return A buffer with the contents of this buffer from the index, avoiding copies if possible, but with + * no shared internal buffers. */ default RetainableByteBuffer take(long fromIndex) { @@ -386,7 +388,9 @@ default RetainableByteBuffer take(long fromIndex) RetainableByteBuffer slice = slice(); limit(fromIndex); slice.skip(fromIndex); - return slice; + RetainableByteBuffer copy = slice.copy(); + slice.release(); + return copy; } /** @@ -1436,23 +1440,39 @@ public RetainableByteBuffer take(long fromIndex) List buffers = new ArrayList<>(_buffers.size()); _aggregate = null; - for (Iterator i = _buffers.iterator(); i.hasNext();) + for (ListIterator i = _buffers.listIterator(); i.hasNext();) { RetainableByteBuffer buffer = i.next(); long size = buffer.size(); if (fromIndex >= size) { + // the sub buffer stays with this RBB fromIndex -= size; } else if (fromIndex == 0) { + // the sub buffer is added to the new RBB i.remove(); buffers.add(buffer); } else { - buffers.add(buffer.take(fromIndex)); + // the sub buffer is split between this RBB and the new RBB + if (fromIndex > (buffer.size() / 2)) + { + // copy only the small part at the end + buffers.add(buffer.take(fromIndex)); + } + else + { + // copy only the small part at the beginning + RetainableByteBuffer slice = buffer.slice(); + slice.limit(fromIndex); + i.set(slice.copy()); + buffer.skip(fromIndex); + buffers.add(buffer); + } fromIndex = 0; } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java index ede79d9fccc2..022bff25c1be 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java @@ -1237,17 +1237,21 @@ public void testTake(Mutable buffer) public void testTakeFrom(Mutable buffer) { buffer.put("Hello".getBytes(StandardCharsets.UTF_8)); - buffer.put((byte)' '); CountDownLatch released = new CountDownLatch(1); - buffer.add(RetainableByteBuffer.wrap(BufferUtil.toBuffer("cruel ".getBytes(StandardCharsets.UTF_8)), released::countDown)); + buffer.add(RetainableByteBuffer.wrap(BufferUtil.toBuffer(" cruel ".getBytes(StandardCharsets.UTF_8)), released::countDown)); buffer.add(RetainableByteBuffer.wrap(BufferUtil.toBuffer("world!".getBytes(StandardCharsets.UTF_8)), released::countDown)); RetainableByteBuffer space = buffer.take(5); + + RetainableByteBuffer bang = space.take(space.size() - 1); RetainableByteBuffer cruelWorld = space.take(1); + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("Hello")); assertThat(BufferUtil.toString(space.getByteBuffer()), is(" ")); - assertThat(BufferUtil.toString(cruelWorld.getByteBuffer()), is("cruel world!")); + assertThat(BufferUtil.toString(cruelWorld.getByteBuffer()), is("cruel world")); + assertThat(BufferUtil.toString(bang.getByteBuffer()), is("!")); space.release(); cruelWorld.release(); + bang.release(); assertTrue(buffer.release()); }