From 9efeae54271f41b042fd6e19376c91ff8f491059 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 21 Sep 2023 20:57:22 +1000 Subject: [PATCH 01/15] An omnibus PR for changes needed to support webfunctions Web functions are currently supported with servlets. These changes add/move utility classes to core to better support direct usage of core APIs --- .../jetty-io/src/main/java/module-info.java | 1 + .../eclipse/jetty/io/ChunkAccumulator.java | 232 ++++++++++++++++++ .../java/org/eclipse/jetty/io/Content.java | 44 +++- .../jetty/io/RetainableByteBuffer.java | 65 ++++- .../io/writer/AbstractOutputStreamWriter.java | 128 ++++++++++ .../jetty/io/writer/EncodingWriter.java | 26 +- .../jetty/io/writer/Iso88591Writer.java | 17 +- .../eclipse/jetty/io/writer/Utf8Writer.java | 19 +- .../jetty/io/writer/AbstractWriterTest.java | 144 ++++------- .../src/main/java/module-info.java | 1 - .../servlet/{writer => }/ResponseWriter.java | 19 +- .../ee10/servlet/ServletApiResponse.java | 16 +- .../ee10/servlet/ServletContextHandler.java | 1 - .../ee10/servlet/ServletContextResponse.java | 1 - .../jetty/ee10/servlet/writer/HttpWriter.java | 77 ------ .../servlet/writer/Iso88591HttpWriter.java | 67 ----- .../ee10/servlet/writer/Utf8HttpWriter.java | 179 -------------- .../jetty/ee9/nested/EncodingHttpWriter.java | 51 ---- .../eclipse/jetty/ee9/nested/HttpWriter.java | 76 ------ .../eclipse/jetty/ee9/nested/Response.java | 10 +- .../jetty/ee9/nested/ResponseWriter.java | 15 +- 21 files changed, 563 insertions(+), 626 deletions(-) create mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java create mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java rename jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/EncodingHttpWriter.java => jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java (63%) rename jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Iso88591HttpWriter.java => jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java (76%) rename jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Utf8HttpWriter.java => jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java (93%) rename jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/HttpWriterTest.java => jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java (59%) rename jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/{writer => }/ResponseWriter.java (95%) delete mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/HttpWriter.java delete mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Iso88591HttpWriter.java delete mode 100644 jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Utf8HttpWriter.java delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/EncodingHttpWriter.java delete mode 100644 jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpWriter.java diff --git a/jetty-core/jetty-io/src/main/java/module-info.java b/jetty-core/jetty-io/src/main/java/module-info.java index 999fd0f0f997..f426e09452dd 100644 --- a/jetty-core/jetty-io/src/main/java/module-info.java +++ b/jetty-core/jetty-io/src/main/java/module-info.java @@ -23,4 +23,5 @@ exports org.eclipse.jetty.io; exports org.eclipse.jetty.io.content; exports org.eclipse.jetty.io.ssl; + exports org.eclipse.jetty.io.writer; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java new file mode 100644 index 000000000000..2fbd879fcec3 --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -0,0 +1,232 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; + +import org.eclipse.jetty.io.Content.Chunk; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.CompletableTask; + +/** + * An accumulator of {@link Content.Chunk}s used to facilitate minimal copy + * aggregation of multiple chunks. + */ +public class ChunkAccumulator +{ + private static final ByteBufferPool NON_POOLING = new ByteBufferPool.NonPooling(); + private final List _chunks = new ArrayList<>(); + private int _size; + + public ChunkAccumulator() + { + } + + /** + * Add a {@link Chunk} to the accumulator. + * @param chunk The {@link Chunk} to accumulate + * @return true if the {@link Chunk} had content and was added to the accumulator. + */ + public boolean add(Chunk chunk) + { + if (chunk.hasRemaining()) + { + _size = Math.addExact(_size, chunk.remaining()); + return _chunks.add(chunk); + } + return false; + } + + /** + * Get the total size of the accumulated {@link Chunk}s. + * @return The total size in bytes. + */ + public int size() + { + return _size; + } + + public byte[] take() + { + byte[] bytes = new byte[_size]; + int offset = 0; + for (Chunk chunk : _chunks) + { + offset += chunk.get(bytes, offset, chunk.remaining()); + chunk.release(); + } + assert offset == _size; + _chunks.clear(); + _size = 0; + return bytes; + } + + public RetainableByteBuffer take(ByteBufferPool pool, boolean direct) + { + if (_size == 0) + return RetainableByteBuffer.EMPTY; + + if (_chunks.size() == 1) + { + Chunk chunk = _chunks.get(0); + ByteBuffer byteBuffer = chunk.getByteBuffer(); + + if (direct == byteBuffer.isDirect()) + { + _chunks.clear(); + _size = 0; + return RetainableByteBuffer.wrap(byteBuffer, chunk); + } + } + + RetainableByteBuffer buffer = Objects.requireNonNullElse(pool, NON_POOLING).acquire(_size, direct); + int offset = 0; + for (Chunk chunk : _chunks) + { + offset += chunk.remaining(); + BufferUtil.append(buffer.getByteBuffer(), chunk.getByteBuffer()); + chunk.release(); + } + assert offset == _size; + _chunks.clear(); + _size = 0; + return buffer; + } + + public void close() + { + _chunks.forEach(Chunk::release); + _chunks.clear(); + _size = 0; + } + + public CompletableFuture readAll(Content.Source source) + { + return readAll(source, -1); + } + + public CompletableFuture readAll(Content.Source source, int maxSize) + { + CompletableTask task = new AccumulatorTask<>(source, maxSize) + { + @Override + protected byte[] take(ChunkAccumulator accumulator) + { + return accumulator.take(); + } + }; + task.run(); + return task; + } + + /** + * @param source The {@link Content.Source} to read + * @param pool The {@link ByteBufferPool} to acquire the buffer from, or null for a non {@link Retainable} buffer + * @param direct True if the buffer should be direct. + * @param maxSize The maximum size to read, or -1 for no limit + * @return A {@link CompletableFuture} that will be completed when the complete content is read or + * failed if the max size is exceeded or there is a read error. + */ + public CompletableFuture readAll(Content.Source source, ByteBufferPool pool, boolean direct, int maxSize) + { + CompletableTask task = new AccumulatorTask<>(source, maxSize) + { + @Override + protected RetainableByteBuffer take(ChunkAccumulator accumulator) + { + return accumulator.take(pool, direct); + } + }; + task.run(); + return task; + } + + private abstract static class AccumulatorTask extends CompletableTask + { + final Content.Source _source; + final ChunkAccumulator _accumulator = new ChunkAccumulator(); + final int _maxSize; + + AccumulatorTask(Content.Source source, int maxSize) + { + _source = source; + _maxSize = maxSize; + } + + @Override + public void run() + { + try + { + while (true) + { + Chunk chunk = _source.read(); + if (chunk == null) + { + _source.demand(this); + return; + } + + if (Chunk.isFailure(chunk)) + { + completeExceptionally(chunk.getFailure()); + return; + } + + if (chunk.hasRemaining()) + { + if (chunk.canRetain()) + _accumulator.add(chunk); + else + { + _accumulator.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast(), () -> + {})); + chunk.release(); + } + + if (_maxSize > 0 && _accumulator._size > _maxSize) + { + _accumulator.close(); + IOException ioe = new IOException("too large"); + _source.fail(ioe); + completeExceptionally(ioe); + return; + } + } + + if (chunk.isLast()) + { + complete(take(_accumulator)); + return; + } + } + } + catch (ArithmeticException e) + { + _accumulator.close(); + IOException ioe = new IOException("too large"); + _source.fail(ioe); + completeExceptionally(ioe); + } + } + + protected abstract T take(ChunkAccumulator accumulator); + + } +} 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 09ae16bdb943..d6ac4abe5d3a 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 @@ -161,6 +161,19 @@ static ByteBuffer asByteBuffer(Source source) throws IOException } } + /** + *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

+ * + * @param source the source to read + * @param maxSize The maximum size to read, or -1 for no limit + * @return A {@link CompletableFuture} that will be completed when the complete content is read or + * failed if the max size is exceeded or there is a read error. + */ + static CompletableFuture asByteArrayAsync(Source source, int maxSize) + { + return new ChunkAccumulator().readAll(source, maxSize); + } + /** *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

* @@ -169,9 +182,34 @@ static ByteBuffer asByteBuffer(Source source) throws IOException */ static CompletableFuture asByteBufferAsync(Source source) { - Promise.Completable completable = new Promise.Completable<>(); - asByteBuffer(source, completable); - return completable; + return asByteBufferAsync(source, -1); + } + + /** + *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

+ * + * @param source the source to read + * @param maxSize The maximum size to read, or -1 for no limit + * @return the {@link CompletableFuture} to notify when the whole content has been read + */ + static CompletableFuture asByteBufferAsync(Source source, int maxSize) + { + return new ChunkAccumulator().readAll(source, -1).thenApply(ByteBuffer::wrap); + } + + /** + *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

+ * + * @param source The {@link Content.Source} to read + * @param pool The {@link ByteBufferPool} to acquire the buffer from, or null for a non {@link Retainable} buffer + * @param direct True if the buffer should be direct. + * @param maxSize The maximum size to read, or -1 for no limit + * @return A {@link CompletableFuture} that will be completed when the complete content is read or + * failed if the max size is exceeded or there is a read error. + */ + static CompletableFuture asRetainableByteBuffer(Source source, ByteBufferPool pool, boolean direct, int maxSize) + { + return new ChunkAccumulator().readAll(source, pool, direct, maxSize); } /** 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 c6ed33088dce..78e954833502 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 @@ -39,7 +39,7 @@ public interface RetainableByteBuffer extends Retainable /** * A Zero-capacity, non-retainable {@code RetainableByteBuffer}. */ - public static RetainableByteBuffer EMPTY = wrap(BufferUtil.EMPTY_BUFFER); + RetainableByteBuffer EMPTY = wrap(BufferUtil.EMPTY_BUFFER); /** *

Returns a non-retainable {@code RetainableByteBuffer} that wraps @@ -57,27 +57,72 @@ public interface RetainableByteBuffer extends Retainable * @return a non-retainable {@code RetainableByteBuffer} * @see ByteBufferPool.NonPooling */ - public static RetainableByteBuffer wrap(ByteBuffer byteBuffer) + static RetainableByteBuffer wrap(ByteBuffer byteBuffer) { return new NonRetainableByteBuffer(byteBuffer); } + /** + *

Returns a {@code RetainableByteBuffer} that wraps + * the given {@code ByteBuffer} and {@link Retainable}.

+ * + * @param byteBuffer the {@code ByteBuffer} to wrap + * @param retainable the associated {@link Retainable}. + * @return a {@code RetainableByteBuffer} + * @see ByteBufferPool.NonPooling + */ + static RetainableByteBuffer wrap(ByteBuffer byteBuffer, Retainable retainable) + { + return new RetainableByteBuffer() + { + @Override + public ByteBuffer getByteBuffer() + { + return byteBuffer; + } + + @Override + public boolean isRetained() + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean canRetain() + { + return retainable.canRetain(); + } + + @Override + public void retain() + { + retainable.retain(); + } + + @Override + public boolean release() + { + return retainable.release(); + } + }; + } + /** * @return whether this instance is retained * @see ReferenceCounter#isRetained() */ - public boolean isRetained(); + boolean isRetained(); /** * Get the wrapped, not {@code null}, {@code ByteBuffer}. * @return the wrapped, not {@code null}, {@code ByteBuffer} */ - public ByteBuffer getByteBuffer(); + ByteBuffer getByteBuffer(); /** * @return whether the {@code ByteBuffer} is direct */ - public default boolean isDirect() + default boolean isDirect() { return getByteBuffer().isDirect(); } @@ -85,7 +130,7 @@ public default boolean isDirect() /** * @return the number of remaining bytes in the {@code ByteBuffer} */ - public default int remaining() + default int remaining() { return getByteBuffer().remaining(); } @@ -93,7 +138,7 @@ public default int remaining() /** * @return whether the {@code ByteBuffer} has remaining bytes */ - public default boolean hasRemaining() + default boolean hasRemaining() { return getByteBuffer().hasRemaining(); } @@ -101,7 +146,7 @@ public default boolean hasRemaining() /** * @return the {@code ByteBuffer} capacity */ - public default int capacity() + default int capacity() { return getByteBuffer().capacity(); } @@ -109,7 +154,7 @@ public default int capacity() /** * @see BufferUtil#clear(ByteBuffer) */ - public default void clear() + default void clear() { BufferUtil.clear(getByteBuffer()); } @@ -117,7 +162,7 @@ public default void clear() /** * A wrapper for {@link RetainableByteBuffer} instances */ - public class Wrapper extends Retainable.Wrapper implements RetainableByteBuffer + class Wrapper extends Retainable.Wrapper implements RetainableByteBuffer { public Wrapper(RetainableByteBuffer wrapped) { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java new file mode 100644 index 000000000000..6db5b88735d9 --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java @@ -0,0 +1,128 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io.writer; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.Writer; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; + +import org.eclipse.jetty.util.ByteArrayOutputStream2; + +/** + * An alternate to {@link java.io.OutputStreamWriter} that supports + * several optimized implementation for well known {@link Charset}s, + * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}. + */ +public abstract class AbstractOutputStreamWriter extends Writer +{ + /** + * Obtain a new {@link Writer} that converts characters written to bytes + * written to an {@link OutputStream}. + * @param outputStream The {@link OutputStream} to write to/ + * @param charset The {@link Charset} name. + * @return A Writer that will + * @throws IOException If there is a problem creating the {@link Writer}. + */ + public static Writer newWriter(OutputStream outputStream, String charset) + throws IOException + { + if (StandardCharsets.ISO_8859_1.name().equalsIgnoreCase(charset)) + return new Iso88591Writer(outputStream); + if (StandardCharsets.UTF_8.name().equalsIgnoreCase(charset)) + return new Iso88591Writer(outputStream); + return new EncodingWriter(outputStream, charset); + } + + /** + * Obtain a new {@link Writer} that converts characters written to bytes + * written to an {@link OutputStream}. + * @param outputStream The {@link OutputStream} to write to/ + * @param charset The {@link Charset}. + * @return A Writer that will + * @throws IOException If there is a problem creating the {@link Writer}. + */ + public static Writer newWriter(OutputStream outputStream, Charset charset) + throws IOException + { + if (StandardCharsets.ISO_8859_1 == charset) + return new Iso88591Writer(outputStream); + if (StandardCharsets.UTF_8.equals(charset)) + return new Iso88591Writer(outputStream); + return new EncodingWriter(outputStream, charset); + } + + protected final int _maxWriteSize; + protected final OutputStream _out; + protected final ByteArrayOutputStream2 _bytes; + protected final char[] _chars; + + protected AbstractOutputStreamWriter(OutputStream out) + { + this(out, 0); + } + + /** + * Construct an {@link java.io.OutputStreamWriter} + * @param out The {@link OutputStream} to write the converted bytes to. + * @param maxWriteSize The maximum size in characters of a single conversion + */ + protected AbstractOutputStreamWriter(OutputStream out, int maxWriteSize) + { + _maxWriteSize = maxWriteSize <= 0 ? 1024 : maxWriteSize; + _out = out; + _chars = new char[_maxWriteSize]; + _bytes = new ByteArrayOutputStream2(_maxWriteSize); + } + + public OutputStream getOutputStream() + { + return _out; + } + + public int getMaxWriteSize() + { + return _maxWriteSize; + } + + @Override + public void close() throws IOException + { + _out.close(); + } + + @Override + public void flush() throws IOException + { + _out.flush(); + } + + @Override + public void write(String s, int offset, int length) throws IOException + { + while (length > _maxWriteSize) + { + write(s, offset, _maxWriteSize); + offset += _maxWriteSize; + length -= _maxWriteSize; + } + + s.getChars(offset, offset + length, _chars, 0); + write(_chars, 0, length); + } + + @Override + public abstract void write(char[] s, int offset, int length) throws IOException; +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/EncodingHttpWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java similarity index 63% rename from jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/EncodingHttpWriter.java rename to jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java index 667480e18584..36760e55af5b 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/EncodingHttpWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java @@ -11,41 +11,45 @@ // ======================================================================== // -package org.eclipse.jetty.ee10.servlet.writer; +package org.eclipse.jetty.io.writer; import java.io.IOException; +import java.io.OutputStream; import java.io.OutputStreamWriter; -import java.io.UnsupportedEncodingException; import java.io.Writer; - -import org.eclipse.jetty.ee10.servlet.HttpOutput; +import java.nio.charset.Charset; /** - * + * An implementation of {@link AbstractOutputStreamWriter} that internally + * uses {@link java.io.OutputStreamWriter}. */ -public class EncodingHttpWriter extends HttpWriter +public class EncodingWriter extends AbstractOutputStreamWriter { final Writer _converter; - public EncodingHttpWriter(HttpOutput out, String encoding) throws IOException + public EncodingWriter(OutputStream out, String encoding) throws IOException { super(out); _converter = new OutputStreamWriter(_bytes, encoding); } + public EncodingWriter(OutputStream out, Charset charset) throws IOException + { + super(out); + _converter = new OutputStreamWriter(_bytes, charset); + } + @Override public void write(char[] s, int offset, int length) throws IOException { - HttpOutput out = _out; - while (length > 0) { _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); + int chars = Math.min(length, _maxWriteSize); _converter.write(s, offset, chars); _converter.flush(); - _bytes.writeTo(out); + _bytes.writeTo(_out); length -= chars; offset += chars; } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Iso88591HttpWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java similarity index 76% rename from jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Iso88591HttpWriter.java rename to jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java index 6186baeb0794..6bab57f9115f 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Iso88591HttpWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java @@ -11,17 +11,20 @@ // ======================================================================== // -package org.eclipse.jetty.ee9.nested; +package org.eclipse.jetty.io.writer; import java.io.IOException; +import java.io.OutputStream; /** - * + * An implementation of {@link AbstractOutputStreamWriter} for + * optimal ISO-8859-1 conversion. + * The ISO-8859-1 encoding is done by this class and no additional + * buffers or Writers are used. */ -public class Iso88591HttpWriter extends HttpWriter +public class Iso88591Writer extends AbstractOutputStreamWriter { - - public Iso88591HttpWriter(HttpOutput out) + public Iso88591Writer(OutputStream out) { super(out); } @@ -29,7 +32,7 @@ public Iso88591HttpWriter(HttpOutput out) @Override public void write(char[] s, int offset, int length) throws IOException { - HttpOutput out = _out; + OutputStream out = _out; if (length == 1) { @@ -41,7 +44,7 @@ public void write(char[] s, int offset, int length) throws IOException while (length > 0) { _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); + int chars = Math.min(length, _maxWriteSize); byte[] buffer = _bytes.getBuf(); int bytes = _bytes.getCount(); diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Utf8HttpWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java similarity index 93% rename from jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Utf8HttpWriter.java rename to jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java index 2b56c039899e..743ea62212fd 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Utf8HttpWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java @@ -11,24 +11,23 @@ // ======================================================================== // -package org.eclipse.jetty.ee9.nested; +package org.eclipse.jetty.io.writer; import java.io.IOException; +import java.io.OutputStream; /** - * OutputWriter. - * A writer that can wrap a {@link HttpOutput} stream and provide - * character encodings. - * + * An implementation of {@link AbstractOutputStreamWriter} for + * an optimal UTF-8 conversion. * The UTF-8 encoding is done by this class and no additional * buffers or Writers are used. - * The UTF-8 code was inspired by http://javolution.org + * The UTF-8 code was inspired by ... */ -public class Utf8HttpWriter extends HttpWriter +public class Utf8Writer extends AbstractOutputStreamWriter { int _surrogate = 0; - public Utf8HttpWriter(HttpOutput out) + public Utf8Writer(OutputStream out) { super(out); } @@ -36,12 +35,12 @@ public Utf8HttpWriter(HttpOutput out) @Override public void write(char[] s, int offset, int length) throws IOException { - HttpOutput out = _out; + OutputStream out = _out; while (length > 0) { _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); + int chars = Math.min(length, _maxWriteSize); byte[] buffer = _bytes.getBuf(); int bytes = _bytes.getCount(); diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/HttpWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java similarity index 59% rename from jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/HttpWriterTest.java rename to jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java index 5d1334756ad1..8a6fa3d0236a 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/HttpWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java @@ -11,73 +11,38 @@ // ======================================================================== // -package org.eclipse.jetty.ee9.nested; +package org.eclipse.jetty.io.writer; -import java.io.IOException; +import java.io.OutputStream; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import org.eclipse.jetty.http.MimeTypes; -import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.io.ByteBufferOutputStream; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Utf8StringBuilder; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import static org.junit.jupiter.api.Assertions.assertEquals; -@Disabled // TODO -public class HttpWriterTest +public class AbstractWriterTest { - // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck - private HttpOutput _httpOut; + private OutputStream _out; private ByteBuffer _bytes; @BeforeEach public void init() throws Exception { _bytes = BufferUtil.allocate(2048); - - Server server = new Server(); - ContextHandler contextHandler = new ContextHandler(server); - - HttpChannel channel = new HttpChannel(contextHandler, new MockConnectionMetaData()) - { - @Override - public boolean failAllContent(Throwable failure) - { - return false; - } - - @Override - public boolean failed(Throwable x) - { - return false; - } - - @Override - protected boolean eof() - { - return false; - } - }; - - _httpOut = new HttpOutput(channel) - { - @Override - public void write(byte[] b, int off, int len) throws IOException - { - BufferUtil.append(_bytes, b, off, len); - } - }; + _out = new ByteBufferOutputStream(_bytes); } @Test public void testSimpleUTF8() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); + AbstractOutputStreamWriter writer = new Utf8Writer(_out); writer.write("Now is the time"); assertArrayEquals("Now is the time".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @@ -85,24 +50,24 @@ public void testSimpleUTF8() throws Exception @Test public void testUTF8() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); - writer.write("How now \uFF22rown cow"); - assertArrayEquals("How now \uFF22rown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); + AbstractOutputStreamWriter writer = new Utf8Writer(_out); + writer.write("How now Brown cow"); + assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @Test public void testUTF16() throws Exception { - HttpWriter writer = new EncodingHttpWriter(_httpOut, MimeTypes.UTF16); - writer.write("How now \uFF22rown cow"); - assertArrayEquals("How now \uFF22rown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); + AbstractOutputStreamWriter writer = new EncodingWriter(_out, StandardCharsets.UTF_16.displayName()); + writer.write("How now Brown cow"); + assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); } @Test public void testNotCESU8() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); - String data = "xxx\uD801\uDC00xxx"; + AbstractOutputStreamWriter writer = new Utf8Writer(_out); + String data = "xxx𐐀xxx"; writer.write(data); byte[] b = BufferUtil.toArray(_bytes); assertEquals("787878F0909080787878", StringUtil.toHexString(b)); @@ -117,25 +82,20 @@ public void testNotCESU8() throws Exception @Test public void testMultiByteOverflowUTF8() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); + AbstractOutputStreamWriter writer = new Utf8Writer(_out); + int maxWriteSize = writer.getMaxWriteSize(); final String singleByteStr = "a"; - final String multiByteDuplicateStr = "\uFF22"; + final String multiByteDuplicateStr = "B"; int remainSize = 1; int multiByteStrByteLength = multiByteDuplicateStr.getBytes(StandardCharsets.UTF_8).length; StringBuilder sb = new StringBuilder(); - for (int i = 0; i < HttpWriter.MAX_OUTPUT_CHARS - multiByteStrByteLength; i++) - { - sb.append(singleByteStr); - } + sb.append(singleByteStr.repeat(Math.max(0, maxWriteSize - multiByteStrByteLength))); sb.append(multiByteDuplicateStr); - for (int i = 0; i < remainSize; i++) - { - sb.append(singleByteStr); - } - char[] buf = new char[HttpWriter.MAX_OUTPUT_CHARS * 3]; + sb.append(singleByteStr.repeat(remainSize)); + char[] buf = new char[maxWriteSize * 3]; - int length = HttpWriter.MAX_OUTPUT_CHARS - multiByteStrByteLength + remainSize + 1; + int length = maxWriteSize - multiByteStrByteLength + remainSize + 1; sb.toString().getChars(0, length, buf, 0); writer.write(buf, 0, length); @@ -146,17 +106,16 @@ public void testMultiByteOverflowUTF8() throws Exception @Test public void testISO8859() throws Exception { - HttpWriter writer = new Iso88591HttpWriter(_httpOut); - writer.write("How now \uFF22rown cow"); + AbstractOutputStreamWriter writer = new Iso88591Writer(_out); + writer.write("How now Brown cow"); assertEquals(new String(BufferUtil.toArray(_bytes), StandardCharsets.ISO_8859_1), "How now ?rown cow"); } @Test public void testUTF16x2() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); - - String source = "\uD842\uDF9F"; + AbstractOutputStreamWriter writer = new Utf8Writer(_out); + String source = "𠮟"; byte[] bytes = source.getBytes(StandardCharsets.UTF_8); writer.write(source.toCharArray(), 0, source.toCharArray().length); @@ -177,24 +136,17 @@ public void testUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16x2() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); + AbstractOutputStreamWriter writer = new Utf8Writer(_out); final String singleByteStr = "a"; int remainSize = 1; - final String multiByteDuplicateStr = "\uD842\uDF9F"; + final String multiByteDuplicateStr = "𠮟"; int adjustSize = -1; - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < HttpWriter.MAX_OUTPUT_CHARS + adjustSize; i++) - { - sb.append(singleByteStr); - } - sb.append(multiByteDuplicateStr); - for (int i = 0; i < remainSize; i++) - { - sb.append(singleByteStr); - } - String source = sb.toString(); + String source = + singleByteStr.repeat(Math.max(0, writer.getMaxWriteSize() + adjustSize)) + + multiByteDuplicateStr + + singleByteStr.repeat(remainSize); byte[] bytes = source.getBytes(StandardCharsets.UTF_8); writer.write(source.toCharArray(), 0, source.toCharArray().length); @@ -215,24 +167,17 @@ public void testMultiByteOverflowUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16X22() throws Exception { - HttpWriter writer = new Utf8HttpWriter(_httpOut); + AbstractOutputStreamWriter writer = new Utf8Writer(_out); final String singleByteStr = "a"; int remainSize = 1; - final String multiByteDuplicateStr = "\uD842\uDF9F"; + final String multiByteDuplicateStr = "𠮟"; int adjustSize = -2; - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < HttpWriter.MAX_OUTPUT_CHARS + adjustSize; i++) - { - sb.append(singleByteStr); - } - sb.append(multiByteDuplicateStr); - for (int i = 0; i < remainSize; i++) - { - sb.append(singleByteStr); - } - String source = sb.toString(); + String source = + singleByteStr.repeat(Math.max(0, writer.getMaxWriteSize() + adjustSize)) + + multiByteDuplicateStr + + singleByteStr.repeat(remainSize); byte[] bytes = source.getBytes(StandardCharsets.UTF_8); writer.write(source.toCharArray(), 0, source.toCharArray().length); @@ -252,11 +197,12 @@ public void testMultiByteOverflowUTF16X22() throws Exception private void myReportBytes(byte[] bytes) throws Exception { -// for (int i = 0; i < bytes.length; i++) -// { -// System.err.format("%s%x",(i == 0)?"[":(i % (HttpWriter.MAX_OUTPUT_CHARS) == 0)?"][":",",bytes[i]); -// } -// System.err.format("]->%s\n",new String(bytes,StringUtil.__UTF8)); + if (LoggerFactory.getLogger(AbstractWriterTest.class).isDebugEnabled()) + { + for (int i = 0; i < bytes.length; i++) + System.err.format("%s%x", (i == 0) ? "[" : (i % 512 == 0) ? "][" : ",", bytes[i]); + System.err.format("]->%s\n", new String(bytes, StandardCharsets.UTF_8)); + } } private void assertArrayEquals(byte[] b1, byte[] b2) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/module-info.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/module-info.java index 1fcc6489c07d..4a38a1ac969e 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/module-info.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/module-info.java @@ -39,7 +39,6 @@ exports org.eclipse.jetty.ee10.servlet.security; exports org.eclipse.jetty.ee10.servlet.security.authentication; exports org.eclipse.jetty.ee10.servlet.util; - exports org.eclipse.jetty.ee10.servlet.writer; exports org.eclipse.jetty.ee10.servlet.jmx to diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/ResponseWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java similarity index 95% rename from jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/ResponseWriter.java rename to jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java index a445fffdfd1c..28088cbb858d 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/ResponseWriter.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java @@ -11,11 +11,12 @@ // ======================================================================== // -package org.eclipse.jetty.ee10.servlet.writer; +package org.eclipse.jetty.ee10.servlet; import java.io.IOException; import java.io.InterruptedIOException; import java.io.PrintWriter; +import java.io.Writer; import java.util.Formatter; import java.util.Locale; @@ -23,6 +24,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,17 +43,17 @@ public class ResponseWriter extends PrintWriter { private static final Logger LOG = LoggerFactory.getLogger(ResponseWriter.class); - private final HttpWriter _httpWriter; + private final Writer _writer; private final Locale _locale; private final String _encoding; private IOException _ioException; private boolean _isClosed = false; private Formatter _formatter; - public ResponseWriter(HttpWriter httpWriter, Locale locale, String encoding) + public ResponseWriter(Writer writer, Locale locale, String encoding) { - super(httpWriter, false); - _httpWriter = httpWriter; + super(writer, false); + _writer = writer; _locale = locale; _encoding = encoding; } @@ -71,7 +73,7 @@ public void reopen() { _isClosed = false; clearError(); - out = _httpWriter; + out = _writer; } } @@ -171,7 +173,10 @@ public void complete(Callback callback) { _isClosed = true; } - _httpWriter.complete(callback); + if (_writer instanceof AbstractOutputStreamWriter abstractWriter && abstractWriter.getOutputStream() instanceof HttpOutput httpOutput) + httpOutput.complete(callback); + else + callback.succeeded(); } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index e5ed3e95b79f..00e3701b3588 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -27,16 +27,12 @@ import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.ee10.servlet.ServletContextHandler.ServletRequestInfo; import org.eclipse.jetty.ee10.servlet.ServletContextHandler.ServletResponseInfo; -import org.eclipse.jetty.ee10.servlet.writer.EncodingHttpWriter; -import org.eclipse.jetty.ee10.servlet.writer.Iso88591HttpWriter; -import org.eclipse.jetty.ee10.servlet.writer.ResponseWriter; -import org.eclipse.jetty.ee10.servlet.writer.Utf8HttpWriter; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.session.ManagedSession; @@ -317,14 +313,8 @@ public PrintWriter getWriter() throws IOException if (writer != null && writer.isFor(locale, encoding)) writer.reopen(); else - { - if (MimeTypes.ISO_8859_1.equalsIgnoreCase(encoding)) - getServletResponseInfo().setWriter(writer = new ResponseWriter(new Iso88591HttpWriter(getServletChannel().getHttpOutput()), locale, encoding)); - else if (MimeTypes.UTF8.equalsIgnoreCase(encoding)) - getServletResponseInfo().setWriter(writer = new ResponseWriter(new Utf8HttpWriter(getServletChannel().getHttpOutput()), locale, encoding)); - else - getServletResponseInfo().setWriter(writer = new ResponseWriter(new EncodingHttpWriter(getServletChannel().getHttpOutput(), encoding), locale, encoding)); - } + getServletResponseInfo().setWriter(writer = new ResponseWriter( + AbstractOutputStreamWriter.newWriter(getServletChannel().getHttpOutput(), encoding), locale, encoding)); // Set the output type at the end, because setCharacterEncoding() checks for it. getServletResponseInfo().setOutputType(ServletContextResponse.OutputType.WRITER); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index 5a391acf6554..65cf9358fc5c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -73,7 +73,6 @@ import org.eclipse.jetty.ee10.servlet.security.ConstraintAware; import org.eclipse.jetty.ee10.servlet.security.ConstraintMapping; import org.eclipse.jetty.ee10.servlet.security.ConstraintSecurityHandler; -import org.eclipse.jetty.ee10.servlet.writer.ResponseWriter; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.security.SecurityHandler; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java index 0fc1a7a350b8..af20361e3c59 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java @@ -23,7 +23,6 @@ import jakarta.servlet.ServletResponseWrapper; import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; -import org.eclipse.jetty.ee10.servlet.writer.ResponseWriter; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/HttpWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/HttpWriter.java deleted file mode 100644 index 2c7bb821d253..000000000000 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/HttpWriter.java +++ /dev/null @@ -1,77 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.ee10.servlet.writer; - -import java.io.IOException; -import java.io.Writer; - -import org.eclipse.jetty.ee10.servlet.HttpOutput; -import org.eclipse.jetty.util.ByteArrayOutputStream2; -import org.eclipse.jetty.util.Callback; - -/** - * - */ -public abstract class HttpWriter extends Writer -{ - public static final int MAX_OUTPUT_CHARS = 512; // TODO should this be configurable? super size is 1024 - - final HttpOutput _out; - final ByteArrayOutputStream2 _bytes; - final char[] _chars; - - public HttpWriter(HttpOutput out) - { - _out = out; - _chars = new char[MAX_OUTPUT_CHARS]; - _bytes = new ByteArrayOutputStream2(MAX_OUTPUT_CHARS); // TODO should this be pooled - or do we just recycle the writer? - } - - @Override - public void close() throws IOException - { - _out.close(); - } - - public void complete(Callback callback) - { - _out.complete(callback); - } - - @Override - public void flush() throws IOException - { - _out.flush(); - } - - @Override - public void write(String s, int offset, int length) throws IOException - { - while (length > MAX_OUTPUT_CHARS) - { - write(s, offset, MAX_OUTPUT_CHARS); - offset += MAX_OUTPUT_CHARS; - length -= MAX_OUTPUT_CHARS; - } - - s.getChars(offset, offset + length, _chars, 0); - write(_chars, 0, length); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - throw new AbstractMethodError(); - } -} diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Iso88591HttpWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Iso88591HttpWriter.java deleted file mode 100644 index 08254de5342b..000000000000 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Iso88591HttpWriter.java +++ /dev/null @@ -1,67 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.ee10.servlet.writer; - -import java.io.IOException; - -import org.eclipse.jetty.ee10.servlet.HttpOutput; - -/** - * - */ -public class Iso88591HttpWriter extends HttpWriter -{ - - public Iso88591HttpWriter(HttpOutput out) - { - super(out); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - HttpOutput out = _out; - - if (length == 1) - { - int c = s[offset]; - out.write(c < 256 ? c : '?'); - return; - } - - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); - - byte[] buffer = _bytes.getBuf(); - int bytes = _bytes.getCount(); - - if (chars > buffer.length - bytes) - chars = buffer.length - bytes; - - for (int i = 0; i < chars; i++) - { - int c = s[offset + i]; - buffer[bytes++] = (byte)(c < 256 ? c : '?'); - } - if (bytes >= 0) - _bytes.setCount(bytes); - - _bytes.writeTo(out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Utf8HttpWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Utf8HttpWriter.java deleted file mode 100644 index 876efb83041e..000000000000 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/writer/Utf8HttpWriter.java +++ /dev/null @@ -1,179 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.ee10.servlet.writer; - -import java.io.IOException; - -import org.eclipse.jetty.ee10.servlet.HttpOutput; - -/** - * OutputWriter. - * A writer that can wrap a {@link HttpOutput} stream and provide - * character encodings. - * - * The UTF-8 encoding is done by this class and no additional - * buffers or Writers are used. - * The UTF-8 code was inspired by http://javolution.org - */ -public class Utf8HttpWriter extends HttpWriter -{ - int _surrogate = 0; - - public Utf8HttpWriter(HttpOutput out) - { - super(out); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - HttpOutput out = _out; - - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); - - byte[] buffer = _bytes.getBuf(); - int bytes = _bytes.getCount(); - - if (bytes + chars > buffer.length) - chars = buffer.length - bytes; - - for (int i = 0; i < chars; i++) - { - int code = s[offset + i]; - - // Do we already have a surrogate? - if (_surrogate == 0) - { - // No - is this char code a surrogate? - if (Character.isHighSurrogate((char)code)) - { - _surrogate = code; // UCS-? - continue; - } - } - // else handle a low surrogate - else if (Character.isLowSurrogate((char)code)) - { - code = Character.toCodePoint((char)_surrogate, (char)code); // UCS-4 - } - // else UCS-2 - else - { - code = _surrogate; // UCS-2 - _surrogate = 0; // USED - i--; - } - - if ((code & 0xffffff80) == 0) - { - // 1b - if (bytes >= buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(code); - } - else - { - if ((code & 0xfffff800) == 0) - { - // 2b - if (bytes + 2 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xc0 | (code >> 6)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xffff0000) == 0) - { - // 3b - if (bytes + 3 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xe0 | (code >> 12)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xff200000) == 0) - { - // 4b - if (bytes + 4 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xf0 | (code >> 18)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xf4000000) == 0) - { - // 5b - if (bytes + 5 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xf8 | (code >> 24)); - buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0x80000000) == 0) - { - // 6b - if (bytes + 6 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xfc | (code >> 30)); - buffer[bytes++] = (byte)(0x80 | ((code >> 24) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else - { - buffer[bytes++] = (byte)('?'); - } - - _surrogate = 0; // USED - - if (bytes == buffer.length) - { - chars = i + 1; - break; - } - } - } - _bytes.setCount(bytes); - - _bytes.writeTo(out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/EncodingHttpWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/EncodingHttpWriter.java deleted file mode 100644 index 7609f6d67a2d..000000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/EncodingHttpWriter.java +++ /dev/null @@ -1,51 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.ee9.nested; - -import java.io.IOException; -import java.io.OutputStreamWriter; -import java.io.UnsupportedEncodingException; -import java.io.Writer; - -/** - * - */ -public class EncodingHttpWriter extends HttpWriter -{ - final Writer _converter; - - public EncodingHttpWriter(HttpOutput out, String encoding) throws IOException - { - super(out); - _converter = new OutputStreamWriter(_bytes, encoding); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - HttpOutput out = _out; - - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, MAX_OUTPUT_CHARS); - - _converter.write(s, offset, chars); - _converter.flush(); - _bytes.writeTo(out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpWriter.java deleted file mode 100644 index f5a7d91ef1b7..000000000000 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/HttpWriter.java +++ /dev/null @@ -1,76 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.ee9.nested; - -import java.io.IOException; -import java.io.Writer; - -import org.eclipse.jetty.util.ByteArrayOutputStream2; -import org.eclipse.jetty.util.Callback; - -/** - * - */ -public abstract class HttpWriter extends Writer -{ - public static final int MAX_OUTPUT_CHARS = 512; // TODO should this be configurable? super size is 1024 - - final HttpOutput _out; - final ByteArrayOutputStream2 _bytes; - final char[] _chars; - - public HttpWriter(HttpOutput out) - { - _out = out; - _chars = new char[MAX_OUTPUT_CHARS]; - _bytes = new ByteArrayOutputStream2(MAX_OUTPUT_CHARS); // TODO should this be pooled - or do we just recycle the writer? - } - - @Override - public void close() throws IOException - { - _out.close(); - } - - public void complete(Callback callback) - { - _out.complete(callback); - } - - @Override - public void flush() throws IOException - { - _out.flush(); - } - - @Override - public void write(String s, int offset, int length) throws IOException - { - while (length > MAX_OUTPUT_CHARS) - { - write(s, offset, MAX_OUTPUT_CHARS); - offset += MAX_OUTPUT_CHARS; - length -= MAX_OUTPUT_CHARS; - } - - s.getChars(offset, offset + length, _chars, 0); - write(_chars, 0, length); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - throw new AbstractMethodError(); - } -} diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index 893e68726610..06a163df59b3 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -51,6 +51,7 @@ import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.content.HttpContent; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.HttpCookieUtils; import org.eclipse.jetty.server.HttpCookieUtils.SetCookieHttpField; @@ -869,14 +870,7 @@ public PrintWriter getWriter() throws IOException if (_writer != null && _writer.isFor(locale, encoding)) _writer.reopen(); else - { - if (MimeTypes.ISO_8859_1.equalsIgnoreCase(encoding)) - _writer = new ResponseWriter(new Iso88591HttpWriter(_out), locale, encoding); - else if (MimeTypes.UTF8.equalsIgnoreCase(encoding)) - _writer = new ResponseWriter(new Utf8HttpWriter(_out), locale, encoding); - else - _writer = new ResponseWriter(new EncodingHttpWriter(_out, encoding), locale, encoding); - } + _writer = new ResponseWriter(AbstractOutputStreamWriter.newWriter(_out, encoding), locale, encoding); // Set the output type at the end, because setCharacterEncoding() checks for it. _outputType = OutputType.WRITER; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java index 775ba628bf63..68d8cd895ac1 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java @@ -16,12 +16,14 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.PrintWriter; +import java.io.Writer; import java.util.Formatter; import java.util.Locale; import jakarta.servlet.ServletResponse; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,17 +42,17 @@ public class ResponseWriter extends PrintWriter { private static final Logger LOG = LoggerFactory.getLogger(ResponseWriter.class); - private final HttpWriter _httpWriter; + private final Writer _writer; private final Locale _locale; private final String _encoding; private IOException _ioException; private boolean _isClosed = false; private Formatter _formatter; - public ResponseWriter(HttpWriter httpWriter, Locale locale, String encoding) + public ResponseWriter(Writer httpWriter, Locale locale, String encoding) { super(httpWriter, false); - _httpWriter = httpWriter; + _writer = httpWriter; _locale = locale; _encoding = encoding; } @@ -70,7 +72,7 @@ protected void reopen() { _isClosed = false; clearError(); - out = _httpWriter; + out = _writer; } } @@ -170,7 +172,10 @@ public void complete(Callback callback) { _isClosed = true; } - _httpWriter.complete(callback); + if (_writer instanceof AbstractOutputStreamWriter abstractWriter && abstractWriter.getOutputStream() instanceof HttpOutput httpOutput) + httpOutput.complete(callback); + else + callback.succeeded(); } @Override From b27de9127614dc6506719141a51be2134eb364cf Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 22 Sep 2023 08:27:33 +1000 Subject: [PATCH 02/15] WIP increase usage of Charset in request --- .../org/eclipse/jetty/http/MimeTypes.java | 68 +++++++++++++++---- .../org/eclipse/jetty/server/Request.java | 8 ++- .../jetty/ee10/servlet/ServletApiRequest.java | 45 +++++------- 3 files changed, 80 insertions(+), 41 deletions(-) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java index 237047fc5066..4358f5a1ff78 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java @@ -28,6 +28,7 @@ import java.util.Objects; import java.util.Properties; import java.util.Set; +import java.util.stream.Collectors; import org.eclipse.jetty.util.FileID; import org.eclipse.jetty.util.Index; @@ -300,9 +301,14 @@ public static Charset getKnownCharset(String charsetName) throws UnsupportedEnco } } + public static String nameOf(Charset charset) + { + return charset == null ? null : charset.name(); + } + protected final Map _mimeMap = new HashMap<>(); - protected final Map _inferredEncodings = new HashMap<>(); - protected final Map _assumedEncodings = new HashMap<>(); + protected final Map _inferredEncodings = new HashMap<>(); + protected final Map _assumedEncodings = new HashMap<>(); public MimeTypes() { @@ -314,11 +320,37 @@ public MimeTypes(MimeTypes defaults) if (defaults != null) { _mimeMap.putAll(defaults.getMimeMap()); - _assumedEncodings.putAll(defaults.getAssumedMap()); - _inferredEncodings.putAll(defaults.getInferredMap()); + _assumedEncodings.putAll(defaults._assumedEncodings); + _inferredEncodings.putAll(defaults._inferredEncodings); } } + /** + * Get the explicit, assumed, or inferred Charset for a mime type + * @param mimeType String form or a mimeType + * @return A {@link Charset} or null; + */ + public Charset getCharset(String mimeType) + { + if (mimeType == null) + return null; + + MimeTypes.Type mime = MimeTypes.CACHE.get(mimeType); + if (mime != null && mime.getCharset() != null) + return mime.getCharset(); + + String charsetName = MimeTypes.getCharsetFromContentType(mimeType); + if (charsetName != null) + return Charset.forName(charsetName); + + Charset charset = getAssumedCharset(mimeType); + if (charset != null) + return charset; + + charset = getInferredCharset(mimeType); + return charset; + } + /** * Get the MIME type by filename extension. * @@ -337,16 +369,26 @@ public String getMimeForExtension(String extension) return _mimeMap.get(extension); } - public String getCharsetInferredFromContentType(String contentType) + public Charset getInferredCharset(String contentType) { return _inferredEncodings.get(contentType); } - public String getCharsetAssumedFromContentType(String contentType) + public Charset getAssumedCharset(String contentType) { return _assumedEncodings.get(contentType); } + public String getCharsetInferredFromContentType(String contentType) + { + return nameOf(_inferredEncodings.get(contentType)); + } + + public String getCharsetAssumedFromContentType(String contentType) + { + return nameOf(_assumedEncodings.get(contentType)); + } + public Map getMimeMap() { return Collections.unmodifiableMap(_mimeMap); @@ -354,12 +396,12 @@ public Map getMimeMap() public Map getInferredMap() { - return Collections.unmodifiableMap(_inferredEncodings); + return _inferredEncodings.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().name())); } public Map getAssumedMap() { - return Collections.unmodifiableMap(_assumedEncodings); + return _assumedEncodings.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().name())); } public static class Mutable extends MimeTypes @@ -390,12 +432,12 @@ public String addMimeMapping(String extension, String type) public String addInferred(String contentType, String encoding) { - return _inferredEncodings.put(contentType, encoding); + return nameOf(_inferredEncodings.put(contentType, Charset.forName(encoding))); } public String addAssumed(String contentType, String encoding) { - return _assumedEncodings.put(contentType, encoding); + return nameOf(_assumedEncodings.put(contentType, Charset.forName(encoding))); } } @@ -479,7 +521,7 @@ public Map getAssumedMap() for (Type type : Type.values()) { if (type.isCharsetAssumed()) - _assumedEncodings.put(type.asString(), type.getCharsetString()); + _assumedEncodings.put(type.asString(), type.getCharset()); } String resourceName = "mime.properties"; @@ -548,9 +590,9 @@ else if (_mimeMap.size() < props.keySet().size()) { String charset = props.getProperty(t); if (charset.startsWith("-")) - _assumedEncodings.put(t, charset.substring(1)); + _assumedEncodings.put(t, Charset.forName(charset.substring(1))); else - _inferredEncodings.put(t, props.getProperty(t)); + _inferredEncodings.put(t, Charset.forName(props.getProperty(t))); }); if (_inferredEncodings.isEmpty()) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 963ccb1b0587..c8fa53efedd0 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; @@ -495,12 +496,17 @@ static List getLocales(Request request) }; } - // TODO: consider inline and remove. static InputStream asInputStream(Request request) { return Content.Source.asInputStream(request); } + static Charset getCharset(Request request) + { + String contentType = request.getHeaders().get(HttpHeader.CONTENT_TYPE); + return Objects.requireNonNullElse(request.getContext().getMimeTypes(), MimeTypes.DEFAULTS).getCharset(contentType); + } + static Fields extractQueryParameters(Request request) { String query = request.getHttpURI().getQuery(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java index 0ca45502162c..b051b89cc34c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java @@ -103,10 +103,10 @@ public class ServletApiRequest implements HttpServletRequest private final ServletContextRequest _servletContextRequest; private final ServletChannel _servletChannel; private AsyncContextState _async; - private String _characterEncoding; + private Charset _charset; + private Charset _readerCharset; private int _inputState = ServletContextRequest.INPUT_NONE; private BufferedReader _reader; - private String _readerEncoding; private String _contentType; private boolean _contentParamsExtracted; private Fields _contentParameters; @@ -717,24 +717,13 @@ public Enumeration getAttributeNames() @Override public String getCharacterEncoding() { - if (_characterEncoding == null) - { - if (getRequest().getContext() != null) - _characterEncoding = getServletRequestInfo().getServletContext().getServletContext().getRequestCharacterEncoding(); + if (_charset == null) + _charset = Request.getCharset(getRequest()); - if (_characterEncoding == null) - { - String contentType = getContentType(); - if (contentType != null) - { - MimeTypes.Type mime = MimeTypes.CACHE.get(contentType); - String charset = (mime == null || mime.getCharset() == null) ? MimeTypes.getCharsetFromContentType(contentType) : mime.getCharset().toString(); - if (charset != null) - _characterEncoding = charset; - } - } - } - return _characterEncoding; + if (_charset == null) + return getServletRequestInfo().getServletContext().getServletContext().getRequestCharacterEncoding(); + + return _charset.name(); } @Override @@ -742,8 +731,7 @@ public void setCharacterEncoding(String encoding) throws UnsupportedEncodingExce { if (_inputState != ServletContextRequest.INPUT_NONE) return; - MimeTypes.getKnownCharset(encoding); - _characterEncoding = encoding; + _charset = MimeTypes.getKnownCharset(encoding); } @Override @@ -1039,15 +1027,18 @@ public BufferedReader getReader() throws IOException if (_inputState == ServletContextRequest.INPUT_READER) return _reader; - String encoding = getCharacterEncoding(); - if (encoding == null) - encoding = MimeTypes.ISO_8859_1; + if (_charset == null) + _charset = Request.getCharset(getRequest()); + if (_charset == null) + _charset = getRequest().getContext().getMimeTypes().getCharset(getServletRequestInfo().getServletContext().getServletContextHandler().getDefaultRequestCharacterEncoding()); + if (_charset == null) + _charset = StandardCharsets.ISO_8859_1; - if (_reader == null || !encoding.equalsIgnoreCase(_readerEncoding)) + if (_reader == null || !_charset.equals(_readerCharset)) { ServletInputStream in = getInputStream(); - _readerEncoding = encoding; - _reader = new BufferedReader(new InputStreamReader(in, encoding)) + _readerCharset = _charset; + _reader = new BufferedReader(new InputStreamReader(in, _charset)) { @Override public void close() throws IOException From 69e8276e580876c207623230c870b6b361b9ab39 Mon Sep 17 00:00:00 2001 From: gregw Date: Fri, 22 Sep 2023 22:19:09 +1000 Subject: [PATCH 03/15] Updates --- .../org/eclipse/jetty/http/MimeTypesTest.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MimeTypesTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MimeTypesTest.java index 291ad315265d..f299e8f09b31 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MimeTypesTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MimeTypesTest.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.http; +import java.nio.charset.StandardCharsets; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -21,6 +22,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNull; @@ -159,15 +161,15 @@ public void testWrapper() assertThat(wrapper.getAssumedMap().size(), is(0)); wrapper.addMimeMapping("txt", "text/plain"); - wrapper.addInferred("text/plain", "usascii"); + wrapper.addInferred("text/plain", "us-ascii"); wrapper.addAssumed("json", "utf-8"); assertThat(wrapper.getMimeMap().size(), is(1)); assertThat(wrapper.getInferredMap().size(), is(1)); assertThat(wrapper.getAssumedMap().size(), is(1)); assertThat(wrapper.getMimeByExtension("fee.txt"), is("text/plain")); - assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), is("usascii")); - assertThat(wrapper.getCharsetAssumedFromContentType("json"), is("utf-8")); + assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), equalToIgnoringCase("us-ascii")); + assertThat(wrapper.getCharsetAssumedFromContentType("json"), equalToIgnoringCase("utf-8")); MimeTypes.Mutable wrapped = new MimeTypes.Mutable(null); wrapper.setWrapped(wrapped); @@ -176,23 +178,23 @@ public void testWrapper() assertThat(wrapper.getInferredMap().size(), is(1)); assertThat(wrapper.getAssumedMap().size(), is(1)); assertThat(wrapper.getMimeByExtension("fee.txt"), is("text/plain")); - assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), is("usascii")); - assertThat(wrapper.getCharsetAssumedFromContentType("json"), is("utf-8")); + assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), equalToIgnoringCase("us-ascii")); + assertThat(wrapper.getCharsetAssumedFromContentType("json"), equalToIgnoringCase("utf-8")); - wrapped.addMimeMapping("txt", "overridden"); - wrapped.addInferred("text/plain", "overridden"); - wrapped.addAssumed("json", "overridden"); + wrapped.addMimeMapping("txt", StandardCharsets.UTF_16.name()); + wrapped.addInferred("text/plain", StandardCharsets.UTF_16.name()); + wrapped.addAssumed("json", StandardCharsets.UTF_16.name()); assertThat(wrapper.getMimeMap().size(), is(1)); assertThat(wrapper.getInferredMap().size(), is(1)); assertThat(wrapper.getAssumedMap().size(), is(1)); assertThat(wrapper.getMimeByExtension("fee.txt"), is("text/plain")); - assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), is("usascii")); - assertThat(wrapper.getCharsetAssumedFromContentType("json"), is("utf-8")); + assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), equalToIgnoringCase("us-ascii")); + assertThat(wrapper.getCharsetAssumedFromContentType("json"), equalToIgnoringCase("utf-8")); wrapped.addMimeMapping("xml", "text/xml"); wrapped.addInferred("text/xml", "iso-8859-1"); - wrapped.addAssumed("text/xxx", "assumed"); + wrapped.addAssumed("text/xxx", StandardCharsets.UTF_16.name()); assertThat(wrapped.getMimeMap().size(), is(2)); assertThat(wrapped.getInferredMap().size(), is(2)); assertThat(wrapped.getAssumedMap().size(), is(2)); @@ -201,10 +203,10 @@ public void testWrapper() assertThat(wrapper.getInferredMap().size(), is(2)); assertThat(wrapper.getAssumedMap().size(), is(2)); assertThat(wrapper.getMimeByExtension("fee.txt"), is("text/plain")); - assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), is("usascii")); - assertThat(wrapper.getCharsetAssumedFromContentType("json"), is("utf-8")); + assertThat(wrapper.getCharsetInferredFromContentType("text/plain"), equalToIgnoringCase("us-ascii")); + assertThat(wrapper.getCharsetAssumedFromContentType("json"), equalToIgnoringCase("utf-8")); assertThat(wrapper.getMimeByExtension("fee.xml"), is("text/xml")); - assertThat(wrapper.getCharsetInferredFromContentType("text/xml"), is("iso-8859-1")); - assertThat(wrapper.getCharsetAssumedFromContentType("text/xxx"), is("assumed")); + assertThat(wrapper.getCharsetInferredFromContentType("text/xml"), equalToIgnoringCase("iso-8859-1")); + assertThat(wrapper.getCharsetAssumedFromContentType("text/xxx"), equalToIgnoringCase("utf-16")); } } From e96029d7d594bbb72f06aee6546f28ad991881e2 Mon Sep 17 00:00:00 2001 From: gregw Date: Sun, 24 Sep 2023 12:03:18 +1000 Subject: [PATCH 04/15] Updates from review --- .../org/eclipse/jetty/http/MimeTypes.java | 2 +- .../jetty-io/src/main/java/module-info.java | 1 - .../jetty/io/AbstractOutputStreamWriter.java | 374 ++++++++++++++++++ .../eclipse/jetty/io/ChunkAccumulator.java | 42 +- .../java/org/eclipse/jetty/io/Content.java | 6 +- .../io/writer/AbstractOutputStreamWriter.java | 128 ------ .../jetty/io/writer/EncodingWriter.java | 57 --- .../jetty/io/writer/Iso88591Writer.java | 68 ---- .../eclipse/jetty/io/writer/Utf8Writer.java | 176 --------- .../jetty/io/writer/AbstractWriterTest.java | 19 +- .../jetty/ee10/servlet/ResponseWriter.java | 2 +- .../ee10/servlet/ServletApiResponse.java | 2 +- .../eclipse/jetty/ee9/nested/Response.java | 2 +- .../jetty/ee9/nested/ResponseWriter.java | 2 +- 14 files changed, 411 insertions(+), 470 deletions(-) create mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java delete mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java delete mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java delete mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java delete mode 100644 jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java index 4358f5a1ff78..c663130beb10 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java @@ -301,7 +301,7 @@ public static Charset getKnownCharset(String charsetName) throws UnsupportedEnco } } - public static String nameOf(Charset charset) + private static String nameOf(Charset charset) { return charset == null ? null : charset.name(); } diff --git a/jetty-core/jetty-io/src/main/java/module-info.java b/jetty-core/jetty-io/src/main/java/module-info.java index f426e09452dd..999fd0f0f997 100644 --- a/jetty-core/jetty-io/src/main/java/module-info.java +++ b/jetty-core/jetty-io/src/main/java/module-info.java @@ -23,5 +23,4 @@ exports org.eclipse.jetty.io; exports org.eclipse.jetty.io.content; exports org.eclipse.jetty.io.ssl; - exports org.eclipse.jetty.io.writer; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java new file mode 100644 index 000000000000..d7ef1701c1f0 --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java @@ -0,0 +1,374 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; + +import org.eclipse.jetty.util.ByteArrayOutputStream2; + +/** + * An alternate to {@link java.io.OutputStreamWriter} that supports + * several optimized implementation for well known {@link Charset}s, + * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}. + */ +public abstract class AbstractOutputStreamWriter extends Writer +{ + private final int _maxWriteSize; + private final char[] _chars; + protected final OutputStream _out; + protected final ByteArrayOutputStream2 _bytes; + + protected AbstractOutputStreamWriter(OutputStream out) + { + this(out, 0); + } + + /** + * Construct an {@link java.io.OutputStreamWriter} + * @param out The {@link OutputStream} to write the converted bytes to. + * @param maxWriteSize The maximum size in characters of a single conversion + */ + protected AbstractOutputStreamWriter(OutputStream out, int maxWriteSize) + { + _maxWriteSize = maxWriteSize <= 0 ? 1024 : maxWriteSize; + _out = out; + _chars = new char[_maxWriteSize]; + _bytes = new ByteArrayOutputStream2(_maxWriteSize); + } + + /** + * Obtain a new {@link Writer} that converts characters written to bytes + * written to an {@link OutputStream}. + * @param outputStream The {@link OutputStream} to write to/ + * @param charset The {@link Charset} name. + * @return A Writer that will + * @throws IOException If there is a problem creating the {@link Writer}. + */ + public static Writer newWriter(OutputStream outputStream, String charset) + throws IOException + { + if (StandardCharsets.ISO_8859_1.name().equalsIgnoreCase(charset)) + return new Iso88591Writer(outputStream); + if (StandardCharsets.UTF_8.name().equalsIgnoreCase(charset)) + return new Utf8Writer(outputStream); + return new EncodingWriter(outputStream, charset); + } + + /** + * Obtain a new {@link Writer} that converts characters written to bytes + * written to an {@link OutputStream}. + * @param outputStream The {@link OutputStream} to write to/ + * @param charset The {@link Charset}. + * @return A Writer that will + * @throws IOException If there is a problem creating the {@link Writer}. + */ + public static Writer newWriter(OutputStream outputStream, Charset charset) + throws IOException + { + if (StandardCharsets.ISO_8859_1 == charset) + return new Iso88591Writer(outputStream); + if (StandardCharsets.UTF_8.equals(charset)) + return new Utf8Writer(outputStream); + return new EncodingWriter(outputStream, charset); + } + + public OutputStream getOutputStream() + { + return _out; + } + + public int getMaxWriteSize() + { + return _maxWriteSize; + } + + @Override + public void close() throws IOException + { + _out.close(); + } + + @Override + public void flush() throws IOException + { + _out.flush(); + } + + @Override + public void write(String string, int offset, int length) throws IOException + { + while (length > _maxWriteSize) + { + int end = offset + _maxWriteSize; + string.getChars(offset, end, _chars, 0); + write(_chars, 0, length); + offset = end; + length -= _maxWriteSize; + } + + string.getChars(offset, offset + length, _chars, 0); + write(_chars, 0, length); + } + + @Override + public abstract void write(char[] s, int offset, int length) throws IOException; + + /** + * An implementation of {@link AbstractOutputStreamWriter} for + * optimal ISO-8859-1 conversion. + * The ISO-8859-1 encoding is done by this class and no additional + * buffers or Writers are used. + */ + public static class Iso88591Writer extends AbstractOutputStreamWriter + { + public Iso88591Writer(OutputStream out) + { + super(out); + } + + @Override + public void write(char[] s, int offset, int length) throws IOException + { + if (length == 1) + { + int c = s[offset]; + _out.write(c < 256 ? c : '?'); + return; + } + + while (length > 0) + { + _bytes.reset(); + int chars = Math.min(length, getMaxWriteSize()); + + byte[] buffer = _bytes.getBuf(); + int bytes = _bytes.getCount(); + + if (chars > buffer.length - bytes) + chars = buffer.length - bytes; + + for (int i = 0; i < chars; i++) + { + int c = s[offset + i]; + buffer[bytes++] = (byte)(c < 256 ? c : '?'); + } + if (bytes >= 0) + _bytes.setCount(bytes); + + _bytes.writeTo(_out); + length -= chars; + offset += chars; + } + } + } + + /** + * An implementation of {@link AbstractOutputStreamWriter} for + * an optimal UTF-8 conversion. + * The UTF-8 encoding is done by this class and no additional + * buffers or Writers are used. + * The UTF-8 code was inspired by ... + */ + public static class Utf8Writer extends AbstractOutputStreamWriter + { + int _surrogate = 0; + + public Utf8Writer(OutputStream out) + { + super(out); + } + + @Override + public void write(char[] s, int offset, int length) throws IOException + { + while (length > 0) + { + _bytes.reset(); + int chars = Math.min(length, getMaxWriteSize()); + + byte[] buffer = _bytes.getBuf(); + int bytes = _bytes.getCount(); + + if (bytes + chars > buffer.length) + chars = buffer.length - bytes; + + for (int i = 0; i < chars; i++) + { + int code = s[offset + i]; + + // Do we already have a surrogate? + if (_surrogate == 0) + { + // No - is this char code a surrogate? + if (Character.isHighSurrogate((char)code)) + { + _surrogate = code; // UCS-? + continue; + } + } + // else handle a low surrogate + else if (Character.isLowSurrogate((char)code)) + { + code = Character.toCodePoint((char)_surrogate, (char)code); // UCS-4 + } + // else UCS-2 + else + { + code = _surrogate; // UCS-2 + _surrogate = 0; // USED + i--; + } + + if ((code & 0xffffff80) == 0) + { + // 1b + if (bytes >= buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(code); + } + else + { + if ((code & 0xfffff800) == 0) + { + // 2b + if (bytes + 2 > buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(0xc0 | (code >> 6)); + buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); + } + else if ((code & 0xffff0000) == 0) + { + // 3b + if (bytes + 3 > buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(0xe0 | (code >> 12)); + buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); + } + else if ((code & 0xff200000) == 0) + { + // 4b + if (bytes + 4 > buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(0xf0 | (code >> 18)); + buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); + } + else if ((code & 0xf4000000) == 0) + { + // 5b + if (bytes + 5 > buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(0xf8 | (code >> 24)); + buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); + } + else if ((code & 0x80000000) == 0) + { + // 6b + if (bytes + 6 > buffer.length) + { + chars = i; + break; + } + buffer[bytes++] = (byte)(0xfc | (code >> 30)); + buffer[bytes++] = (byte)(0x80 | ((code >> 24) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); + buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); + } + else + { + buffer[bytes++] = (byte)('?'); + } + + _surrogate = 0; // USED + + if (bytes == buffer.length) + { + chars = i + 1; + break; + } + } + } + _bytes.setCount(bytes); + + _bytes.writeTo(_out); + length -= chars; + offset += chars; + } + } + } + + /** + * An implementation of {@link AbstractOutputStreamWriter} that internally + * uses {@link java.io.OutputStreamWriter}. + */ + public static class EncodingWriter extends AbstractOutputStreamWriter + { + final Writer _converter; + + public EncodingWriter(OutputStream out, String encoding) throws IOException + { + super(out); + _converter = new OutputStreamWriter(_bytes, encoding); + } + + public EncodingWriter(OutputStream out, Charset charset) throws IOException + { + super(out); + _converter = new OutputStreamWriter(_bytes, charset); + } + + @Override + public void write(char[] s, int offset, int length) throws IOException + { + while (length > 0) + { + _bytes.reset(); + int chars = Math.min(length, getMaxWriteSize()); + + _converter.write(s, offset, chars); + _converter.flush(); + _bytes.writeTo(_out); + length -= chars; + offset += chars; + } + } + } +} diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java index 2fbd879fcec3..3ed3eb9237d3 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -40,7 +40,7 @@ public ChunkAccumulator() /** * Add a {@link Chunk} to the accumulator. - * @param chunk The {@link Chunk} to accumulate + * @param chunk The {@link Chunk} to accumulate. If a reference is kept to the chunk (rather than a copy), it will be retained. * @return true if the {@link Chunk} had content and was added to the accumulator. */ public boolean add(Chunk chunk) @@ -48,7 +48,12 @@ public boolean add(Chunk chunk) if (chunk.hasRemaining()) { _size = Math.addExact(_size, chunk.remaining()); - return _chunks.add(chunk); + if (chunk.canRetain()) + { + chunk.retain(); + return _chunks.add(chunk); + } + return _chunks.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast(), () -> {})); } return false; } @@ -131,8 +136,7 @@ protected byte[] take(ChunkAccumulator accumulator) return accumulator.take(); } }; - task.run(); - return task; + return task.start(); } /** @@ -153,17 +157,16 @@ protected RetainableByteBuffer take(ChunkAccumulator accumulator) return accumulator.take(pool, direct); } }; - task.run(); - return task; + return task.start(); } private abstract static class AccumulatorTask extends CompletableTask { - final Content.Source _source; - final ChunkAccumulator _accumulator = new ChunkAccumulator(); - final int _maxSize; + private final Content.Source _source; + private final ChunkAccumulator _accumulator = new ChunkAccumulator(); + private final int _maxSize; - AccumulatorTask(Content.Source source, int maxSize) + private AccumulatorTask(Content.Source source, int maxSize) { _source = source; _maxSize = maxSize; @@ -191,14 +194,7 @@ public void run() if (chunk.hasRemaining()) { - if (chunk.canRetain()) - _accumulator.add(chunk); - else - { - _accumulator.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast(), () -> - {})); - chunk.release(); - } + _accumulator.add(chunk); if (_maxSize > 0 && _accumulator._size > _maxSize) { @@ -210,6 +206,8 @@ public void run() } } + chunk.release(); + if (chunk.isLast()) { complete(take(_accumulator)); @@ -217,16 +215,14 @@ public void run() } } } - catch (ArithmeticException e) + catch (Throwable e) { _accumulator.close(); - IOException ioe = new IOException("too large"); - _source.fail(ioe); - completeExceptionally(ioe); + _source.fail(e); + completeExceptionally(e); } } protected abstract T take(ChunkAccumulator accumulator); - } } 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 d6ac4abe5d3a..16bd6888839e 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 @@ -162,7 +162,7 @@ static ByteBuffer asByteBuffer(Source source) throws IOException } /** - *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

+ *

Reads, non-blocking, the whole content source into a {@code byte} array.

* * @param source the source to read * @param maxSize The maximum size to read, or -1 for no limit @@ -194,11 +194,11 @@ static CompletableFuture asByteBufferAsync(Source source) */ static CompletableFuture asByteBufferAsync(Source source, int maxSize) { - return new ChunkAccumulator().readAll(source, -1).thenApply(ByteBuffer::wrap); + return asByteArrayAsync(source, maxSize).thenApply(ByteBuffer::wrap); } /** - *

Reads, non-blocking, the whole content source into a {@link ByteBuffer}.

+ *

Reads, non-blocking, the whole content source into a {@link RetainableByteBuffer}.

* * @param source The {@link Content.Source} to read * @param pool The {@link ByteBufferPool} to acquire the buffer from, or null for a non {@link Retainable} buffer diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java deleted file mode 100644 index 6db5b88735d9..000000000000 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/AbstractOutputStreamWriter.java +++ /dev/null @@ -1,128 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.io.writer; - -import java.io.IOException; -import java.io.OutputStream; -import java.io.Writer; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; - -import org.eclipse.jetty.util.ByteArrayOutputStream2; - -/** - * An alternate to {@link java.io.OutputStreamWriter} that supports - * several optimized implementation for well known {@link Charset}s, - * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}. - */ -public abstract class AbstractOutputStreamWriter extends Writer -{ - /** - * Obtain a new {@link Writer} that converts characters written to bytes - * written to an {@link OutputStream}. - * @param outputStream The {@link OutputStream} to write to/ - * @param charset The {@link Charset} name. - * @return A Writer that will - * @throws IOException If there is a problem creating the {@link Writer}. - */ - public static Writer newWriter(OutputStream outputStream, String charset) - throws IOException - { - if (StandardCharsets.ISO_8859_1.name().equalsIgnoreCase(charset)) - return new Iso88591Writer(outputStream); - if (StandardCharsets.UTF_8.name().equalsIgnoreCase(charset)) - return new Iso88591Writer(outputStream); - return new EncodingWriter(outputStream, charset); - } - - /** - * Obtain a new {@link Writer} that converts characters written to bytes - * written to an {@link OutputStream}. - * @param outputStream The {@link OutputStream} to write to/ - * @param charset The {@link Charset}. - * @return A Writer that will - * @throws IOException If there is a problem creating the {@link Writer}. - */ - public static Writer newWriter(OutputStream outputStream, Charset charset) - throws IOException - { - if (StandardCharsets.ISO_8859_1 == charset) - return new Iso88591Writer(outputStream); - if (StandardCharsets.UTF_8.equals(charset)) - return new Iso88591Writer(outputStream); - return new EncodingWriter(outputStream, charset); - } - - protected final int _maxWriteSize; - protected final OutputStream _out; - protected final ByteArrayOutputStream2 _bytes; - protected final char[] _chars; - - protected AbstractOutputStreamWriter(OutputStream out) - { - this(out, 0); - } - - /** - * Construct an {@link java.io.OutputStreamWriter} - * @param out The {@link OutputStream} to write the converted bytes to. - * @param maxWriteSize The maximum size in characters of a single conversion - */ - protected AbstractOutputStreamWriter(OutputStream out, int maxWriteSize) - { - _maxWriteSize = maxWriteSize <= 0 ? 1024 : maxWriteSize; - _out = out; - _chars = new char[_maxWriteSize]; - _bytes = new ByteArrayOutputStream2(_maxWriteSize); - } - - public OutputStream getOutputStream() - { - return _out; - } - - public int getMaxWriteSize() - { - return _maxWriteSize; - } - - @Override - public void close() throws IOException - { - _out.close(); - } - - @Override - public void flush() throws IOException - { - _out.flush(); - } - - @Override - public void write(String s, int offset, int length) throws IOException - { - while (length > _maxWriteSize) - { - write(s, offset, _maxWriteSize); - offset += _maxWriteSize; - length -= _maxWriteSize; - } - - s.getChars(offset, offset + length, _chars, 0); - write(_chars, 0, length); - } - - @Override - public abstract void write(char[] s, int offset, int length) throws IOException; -} diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java deleted file mode 100644 index 36760e55af5b..000000000000 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/EncodingWriter.java +++ /dev/null @@ -1,57 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.io.writer; - -import java.io.IOException; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.Writer; -import java.nio.charset.Charset; - -/** - * An implementation of {@link AbstractOutputStreamWriter} that internally - * uses {@link java.io.OutputStreamWriter}. - */ -public class EncodingWriter extends AbstractOutputStreamWriter -{ - final Writer _converter; - - public EncodingWriter(OutputStream out, String encoding) throws IOException - { - super(out); - _converter = new OutputStreamWriter(_bytes, encoding); - } - - public EncodingWriter(OutputStream out, Charset charset) throws IOException - { - super(out); - _converter = new OutputStreamWriter(_bytes, charset); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, _maxWriteSize); - - _converter.write(s, offset, chars); - _converter.flush(); - _bytes.writeTo(_out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java deleted file mode 100644 index 6bab57f9115f..000000000000 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Iso88591Writer.java +++ /dev/null @@ -1,68 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.io.writer; - -import java.io.IOException; -import java.io.OutputStream; - -/** - * An implementation of {@link AbstractOutputStreamWriter} for - * optimal ISO-8859-1 conversion. - * The ISO-8859-1 encoding is done by this class and no additional - * buffers or Writers are used. - */ -public class Iso88591Writer extends AbstractOutputStreamWriter -{ - public Iso88591Writer(OutputStream out) - { - super(out); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - OutputStream out = _out; - - if (length == 1) - { - int c = s[offset]; - out.write(c < 256 ? c : '?'); - return; - } - - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, _maxWriteSize); - - byte[] buffer = _bytes.getBuf(); - int bytes = _bytes.getCount(); - - if (chars > buffer.length - bytes) - chars = buffer.length - bytes; - - for (int i = 0; i < chars; i++) - { - int c = s[offset + i]; - buffer[bytes++] = (byte)(c < 256 ? c : '?'); - } - if (bytes >= 0) - _bytes.setCount(bytes); - - _bytes.writeTo(out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java deleted file mode 100644 index 743ea62212fd..000000000000 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/writer/Utf8Writer.java +++ /dev/null @@ -1,176 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.io.writer; - -import java.io.IOException; -import java.io.OutputStream; - -/** - * An implementation of {@link AbstractOutputStreamWriter} for - * an optimal UTF-8 conversion. - * The UTF-8 encoding is done by this class and no additional - * buffers or Writers are used. - * The UTF-8 code was inspired by ... - */ -public class Utf8Writer extends AbstractOutputStreamWriter -{ - int _surrogate = 0; - - public Utf8Writer(OutputStream out) - { - super(out); - } - - @Override - public void write(char[] s, int offset, int length) throws IOException - { - OutputStream out = _out; - - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, _maxWriteSize); - - byte[] buffer = _bytes.getBuf(); - int bytes = _bytes.getCount(); - - if (bytes + chars > buffer.length) - chars = buffer.length - bytes; - - for (int i = 0; i < chars; i++) - { - int code = s[offset + i]; - - // Do we already have a surrogate? - if (_surrogate == 0) - { - // No - is this char code a surrogate? - if (Character.isHighSurrogate((char)code)) - { - _surrogate = code; // UCS-? - continue; - } - } - // else handle a low surrogate - else if (Character.isLowSurrogate((char)code)) - { - code = Character.toCodePoint((char)_surrogate, (char)code); // UCS-4 - } - // else UCS-2 - else - { - code = _surrogate; // UCS-2 - _surrogate = 0; // USED - i--; - } - - if ((code & 0xffffff80) == 0) - { - // 1b - if (bytes >= buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(code); - } - else - { - if ((code & 0xfffff800) == 0) - { - // 2b - if (bytes + 2 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xc0 | (code >> 6)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xffff0000) == 0) - { - // 3b - if (bytes + 3 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xe0 | (code >> 12)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xff200000) == 0) - { - // 4b - if (bytes + 4 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xf0 | (code >> 18)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0xf4000000) == 0) - { - // 5b - if (bytes + 5 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xf8 | (code >> 24)); - buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else if ((code & 0x80000000) == 0) - { - // 6b - if (bytes + 6 > buffer.length) - { - chars = i; - break; - } - buffer[bytes++] = (byte)(0xfc | (code >> 30)); - buffer[bytes++] = (byte)(0x80 | ((code >> 24) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 18) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 12) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | ((code >> 6) & 0x3f)); - buffer[bytes++] = (byte)(0x80 | (code & 0x3f)); - } - else - { - buffer[bytes++] = (byte)('?'); - } - - _surrogate = 0; // USED - - if (bytes == buffer.length) - { - chars = i + 1; - break; - } - } - } - _bytes.setCount(bytes); - - _bytes.writeTo(out); - length -= chars; - offset += chars; - } - } -} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java index 8a6fa3d0236a..08a091823051 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java @@ -17,6 +17,7 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.ByteBufferOutputStream; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; @@ -42,7 +43,7 @@ public void init() throws Exception @Test public void testSimpleUTF8() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); writer.write("Now is the time"); assertArrayEquals("Now is the time".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @@ -50,7 +51,7 @@ public void testSimpleUTF8() throws Exception @Test public void testUTF8() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); writer.write("How now Brown cow"); assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @@ -58,7 +59,7 @@ public void testUTF8() throws Exception @Test public void testUTF16() throws Exception { - AbstractOutputStreamWriter writer = new EncodingWriter(_out, StandardCharsets.UTF_16.displayName()); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.EncodingWriter(_out, StandardCharsets.UTF_16.displayName()); writer.write("How now Brown cow"); assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); } @@ -66,7 +67,7 @@ public void testUTF16() throws Exception @Test public void testNotCESU8() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); String data = "xxx𐐀xxx"; writer.write(data); byte[] b = BufferUtil.toArray(_bytes); @@ -82,7 +83,7 @@ public void testNotCESU8() throws Exception @Test public void testMultiByteOverflowUTF8() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); int maxWriteSize = writer.getMaxWriteSize(); final String singleByteStr = "a"; final String multiByteDuplicateStr = "B"; @@ -106,7 +107,7 @@ public void testMultiByteOverflowUTF8() throws Exception @Test public void testISO8859() throws Exception { - AbstractOutputStreamWriter writer = new Iso88591Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Iso88591Writer(_out); writer.write("How now Brown cow"); assertEquals(new String(BufferUtil.toArray(_bytes), StandardCharsets.ISO_8859_1), "How now ?rown cow"); } @@ -114,7 +115,7 @@ public void testISO8859() throws Exception @Test public void testUTF16x2() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); String source = "𠮟"; byte[] bytes = source.getBytes(StandardCharsets.UTF_8); @@ -136,7 +137,7 @@ public void testUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16x2() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); final String singleByteStr = "a"; int remainSize = 1; @@ -167,7 +168,7 @@ public void testMultiByteOverflowUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16X22() throws Exception { - AbstractOutputStreamWriter writer = new Utf8Writer(_out); + AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); final String singleByteStr = "a"; int remainSize = 1; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java index 28088cbb858d..73626e5883ed 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java @@ -22,9 +22,9 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; -import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index 00e3701b3588..cbb7cca363cc 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -32,7 +32,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; +import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.session.ManagedSession; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index 06a163df59b3..f817a8fcf8a8 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -50,8 +50,8 @@ import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.content.HttpContent; +import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.RuntimeIOException; -import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.HttpCookieUtils; import org.eclipse.jetty.server.HttpCookieUtils.SetCookieHttpField; diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java index 68d8cd895ac1..111a73e4959c 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java @@ -21,9 +21,9 @@ import java.util.Locale; import jakarta.servlet.ServletResponse; +import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; -import org.eclipse.jetty.io.writer.AbstractOutputStreamWriter; import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 50bcf06efbae17585530dcb3a256e35bde9cbc5a Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 25 Sep 2023 08:09:02 +1000 Subject: [PATCH 05/15] Updates from review --- .../jetty/io/AbstractOutputStreamWriter.java | 8 +++----- .../eclipse/jetty/ee10/servlet/HttpOutput.java | 13 ++++++++----- .../jetty/ee10/servlet/ResponseWriter.java | 13 ++++++------- .../jetty/ee10/servlet/ServletApiResponse.java | 8 +++++++- .../ee10/servlet/ServletContextResponse.java | 5 ++--- .../org/eclipse/jetty/ee9/nested/Response.java | 15 +++++++++++---- .../eclipse/jetty/ee9/nested/ResponseWriter.java | 11 ++++------- .../eclipse/jetty/ee9/nested/ResponseTest.java | 4 ++-- 8 files changed, 43 insertions(+), 34 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java index d7ef1701c1f0..85cc2e7bc9c1 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java @@ -26,6 +26,9 @@ * An alternate to {@link java.io.OutputStreamWriter} that supports * several optimized implementation for well known {@link Charset}s, * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}. + * The implementations of this class will never buffer characters beyond a call to the + * {@link #write(char[], int, int)} method, thus written characters will always be available + * in converted form to the passed {@link OutputStream}. */ public abstract class AbstractOutputStreamWriter extends Writer { @@ -88,11 +91,6 @@ public static Writer newWriter(OutputStream outputStream, Charset charset) return new EncodingWriter(outputStream, charset); } - public OutputStream getOutputStream() - { - return _out; - } - public int getMaxWriteSize() { return _maxWriteSize; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java index d57452af7eef..6c2fb21921fb 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpOutput.java @@ -321,13 +321,16 @@ public void softClose() } } + /** + * This method is invoked for the COMPLETE action handling in + * HttpChannel.handle. The callback passed typically will call completed + * to finish the request cycle and so may need to asynchronously wait for: + * a pending/blocked operation to finish and then either an async close or + * wait for an application close to complete. + * @param callback The callback to complete when writing the output is complete. + */ public void complete(Callback callback) { - // This method is invoked for the COMPLETE action handling in - // HttpChannel.handle. The callback passed typically will call completed - // to finish the request cycle and so may need to asynchronously wait for: - // a pending/blocked operation to finish and then either an async close or - // wait for an application close to complete. boolean succeeded = false; Throwable error = null; ByteBuffer content = null; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java index 73626e5883ed..927c7c844565 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java @@ -22,10 +22,8 @@ import jakarta.servlet.ServletResponse; import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; -import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -101,7 +99,9 @@ private void setError(Throwable th) super.setError(); if (th instanceof IOException) + { _ioException = (IOException)th; + } else { _ioException = new IOException(String.valueOf(th)); @@ -167,16 +167,15 @@ public void close() } } - public void complete(Callback callback) + /** + * Used to mark this writer as closed during any asynchronous completion operation. + */ + void markAsClosed() { synchronized (lock) { _isClosed = true; } - if (_writer instanceof AbstractOutputStreamWriter abstractWriter && abstractWriter.getOutputStream() instanceof HttpOutput httpOutput) - httpOutput.complete(callback); - else - callback.succeeded(); } @Override diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index cbb7cca363cc..ea3285ad882f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.io.PrintWriter; +import java.io.Writer; import java.util.Collection; import java.util.EnumSet; import java.util.Locale; @@ -313,8 +314,13 @@ public PrintWriter getWriter() throws IOException if (writer != null && writer.isFor(locale, encoding)) writer.reopen(); else + { + // We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters + // in the writer implementation for flush and completion operations. + Writer outputStreamWriter = AbstractOutputStreamWriter.newWriter(getServletChannel().getHttpOutput(), encoding); getServletResponseInfo().setWriter(writer = new ResponseWriter( - AbstractOutputStreamWriter.newWriter(getServletChannel().getHttpOutput(), encoding), locale, encoding)); + outputStreamWriter, locale, encoding)); + } // Set the output type at the end, because setCharacterEncoding() checks for it. getServletResponseInfo().setOutputType(ServletContextResponse.OutputType.WRITER); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java index af20361e3c59..6ed7aa8974a3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextResponse.java @@ -205,9 +205,8 @@ public void included() public void completeOutput(Callback callback) { if (_outputType == OutputType.WRITER) - _writer.complete(callback); - else - getHttpOutput().complete(callback); + _writer.markAsClosed(); + getHttpOutput().complete(callback); } public boolean isAllContentWritten(long written) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index f817a8fcf8a8..a710eb2840c2 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -15,6 +15,7 @@ import java.io.IOException; import java.io.PrintWriter; +import java.io.Writer; import java.nio.channels.IllegalSelectorException; import java.util.Collection; import java.util.Collections; @@ -868,9 +869,16 @@ public PrintWriter getWriter() throws IOException String encoding = getCharacterEncoding(true); Locale locale = getLocale(); if (_writer != null && _writer.isFor(locale, encoding)) + { _writer.reopen(); + } else - _writer = new ResponseWriter(AbstractOutputStreamWriter.newWriter(_out, encoding), locale, encoding); + { + // We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters + // in the writer implementation for flush and completion operations. + Writer outputStreamWriter = AbstractOutputStreamWriter.newWriter(_out, encoding); + _writer = new ResponseWriter(outputStreamWriter, locale, encoding); + } // Set the output type at the end, because setCharacterEncoding() checks for it. _outputType = OutputType.WRITER; @@ -959,9 +967,8 @@ public void completeOutput() throws IOException public void completeOutput(Callback callback) { if (_outputType == OutputType.WRITER) - _writer.complete(callback); - else - _out.complete(callback); + _writer.markAsClosed(); + _out.complete(callback); } public long getLongContentLength() diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java index 111a73e4959c..c7288d30a1d8 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java @@ -21,10 +21,8 @@ import java.util.Locale; import jakarta.servlet.ServletResponse; -import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; -import org.eclipse.jetty.util.Callback; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -166,16 +164,15 @@ public void close() } } - public void complete(Callback callback) + /** + * Used to mark this writer as closed during any asynchronous completion operation. + */ + public void markAsClosed() { synchronized (lock) { _isClosed = true; } - if (_writer instanceof AbstractOutputStreamWriter abstractWriter && abstractWriter.getOutputStream() instanceof HttpOutput httpOutput) - httpOutput.complete(callback); - else - callback.succeeded(); } @Override diff --git a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java index 438a458c3c22..ea48630b44de 100644 --- a/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java +++ b/jetty-ee9/jetty-ee9-nested/src/test/java/org/eclipse/jetty/ee9/nested/ResponseTest.java @@ -318,7 +318,7 @@ public void testAssumedCharset() throws Exception assertEquals("application/vnd.api+json", response.getContentType()); response.getWriter(); assertEquals("application/vnd.api+json", response.getContentType()); - assertEquals("utf-8", response.getCharacterEncoding()); + assertEquals("UTF-8", response.getCharacterEncoding()); } @Test @@ -461,7 +461,7 @@ public void testLocaleAndContentTypeEncoding() throws Exception Response response = getResponse(); response.setContentType("text/html"); - assertEquals("iso-8859-1", response.getCharacterEncoding()); + assertEquals("ISO-8859-1", response.getCharacterEncoding()); // setLocale should change character encoding based on // locale-encoding-mapping-list From 1183e7c27d9b2cead3b937222f4d2d1dff090665 Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 25 Sep 2023 08:56:19 +1000 Subject: [PATCH 06/15] Updates from review --- .../eclipse/jetty/io/ChunkAccumulator.java | 117 +++++++++--------- ...eamWriter.java => WriteThroughWriter.java} | 37 +++--- .../io/{writer => }/AbstractWriterTest.java | 29 +++-- .../jetty/ee10/servlet/ResponseWriter.java | 6 +- .../ee10/servlet/ServletApiResponse.java | 5 +- .../eclipse/jetty/ee9/nested/Response.java | 5 +- .../jetty/ee9/nested/ResponseWriter.java | 6 +- 7 files changed, 99 insertions(+), 106 deletions(-) rename jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/{AbstractOutputStreamWriter.java => WriteThroughWriter.java} (90%) rename jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/{writer => }/AbstractWriterTest.java (85%) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java index 3ed3eb9237d3..bf9fb6ea1450 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -32,22 +32,20 @@ public class ChunkAccumulator { private static final ByteBufferPool NON_POOLING = new ByteBufferPool.NonPooling(); private final List _chunks = new ArrayList<>(); - private int _size; - - public ChunkAccumulator() - { - } + private int _length; /** * Add a {@link Chunk} to the accumulator. * @param chunk The {@link Chunk} to accumulate. If a reference is kept to the chunk (rather than a copy), it will be retained. * @return true if the {@link Chunk} had content and was added to the accumulator. + * @throws ArithmeticException if more that {@link Integer#MAX_VALUE} bytes are added. + * @throws IllegalArgumentException if the passed {@link Chunk} is a {@link Chunk#isFailure(Chunk) failure}. */ public boolean add(Chunk chunk) { if (chunk.hasRemaining()) { - _size = Math.addExact(_size, chunk.remaining()); + _length = Math.addExact(_length, chunk.remaining()); if (chunk.canRetain()) { chunk.retain(); @@ -55,36 +53,40 @@ public boolean add(Chunk chunk) } return _chunks.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast(), () -> {})); } + else if (Chunk.isFailure(chunk)) + { + throw new IllegalArgumentException("chunk is failure"); + } return false; } /** - * Get the total size of the accumulated {@link Chunk}s. - * @return The total size in bytes. + * Get the total length of the accumulated {@link Chunk}s. + * @return The total length in bytes. */ - public int size() + public int length() { - return _size; + return _length; } public byte[] take() { - byte[] bytes = new byte[_size]; + byte[] bytes = new byte[_length]; int offset = 0; for (Chunk chunk : _chunks) { offset += chunk.get(bytes, offset, chunk.remaining()); chunk.release(); } - assert offset == _size; + assert offset == _length; _chunks.clear(); - _size = 0; + _length = 0; return bytes; } public RetainableByteBuffer take(ByteBufferPool pool, boolean direct) { - if (_size == 0) + if (_length == 0) return RetainableByteBuffer.EMPTY; if (_chunks.size() == 1) @@ -95,12 +97,12 @@ public RetainableByteBuffer take(ByteBufferPool pool, boolean direct) if (direct == byteBuffer.isDirect()) { _chunks.clear(); - _size = 0; + _length = 0; return RetainableByteBuffer.wrap(byteBuffer, chunk); } } - RetainableByteBuffer buffer = Objects.requireNonNullElse(pool, NON_POOLING).acquire(_size, direct); + RetainableByteBuffer buffer = Objects.requireNonNullElse(pool, NON_POOLING).acquire(_length, direct); int offset = 0; for (Chunk chunk : _chunks) { @@ -108,9 +110,9 @@ public RetainableByteBuffer take(ByteBufferPool pool, boolean direct) BufferUtil.append(buffer.getByteBuffer(), chunk.getByteBuffer()); chunk.release(); } - assert offset == _size; + assert offset == _length; _chunks.clear(); - _size = 0; + _length = 0; return buffer; } @@ -118,7 +120,7 @@ public void close() { _chunks.forEach(Chunk::release); _chunks.clear(); - _size = 0; + _length = 0; } public CompletableFuture readAll(Content.Source source) @@ -164,63 +166,56 @@ private abstract static class AccumulatorTask extends CompletableTask { private final Content.Source _source; private final ChunkAccumulator _accumulator = new ChunkAccumulator(); - private final int _maxSize; + private final int _maxLength; - private AccumulatorTask(Content.Source source, int maxSize) + private AccumulatorTask(Content.Source source, int maxLength) { _source = source; - _maxSize = maxSize; + _maxLength = maxLength; } @Override public void run() { - try + while (true) { - while (true) + Chunk chunk = _source.read(); + if (chunk == null) + { + _source.demand(this); + break; + } + + if (Chunk.isFailure(chunk)) + { + completeExceptionally(chunk.getFailure()); + break; + } + + try { - Chunk chunk = _source.read(); - if (chunk == null) - { - _source.demand(this); - return; - } - - if (Chunk.isFailure(chunk)) - { - completeExceptionally(chunk.getFailure()); - return; - } - - if (chunk.hasRemaining()) - { - _accumulator.add(chunk); - - if (_maxSize > 0 && _accumulator._size > _maxSize) - { - _accumulator.close(); - IOException ioe = new IOException("too large"); - _source.fail(ioe); - completeExceptionally(ioe); - return; - } - } + _accumulator.add(chunk); + if (_maxLength > 0 && _accumulator._length > _maxLength) + throw new IOException("accumulation too large"); + } + catch (Throwable t) + { chunk.release(); + _accumulator.close(); + _source.fail(t); + completeExceptionally(t); + break; + } - if (chunk.isLast()) - { - complete(take(_accumulator)); - return; - } + chunk.release(); + + if (chunk.isLast()) + { + complete(take(_accumulator)); + break; } } - catch (Throwable e) - { - _accumulator.close(); - _source.fail(e); - completeExceptionally(e); - } } protected abstract T take(ChunkAccumulator accumulator); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java similarity index 90% rename from jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java rename to jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java index 85cc2e7bc9c1..427961cc7c96 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractOutputStreamWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java @@ -23,21 +23,22 @@ import org.eclipse.jetty.util.ByteArrayOutputStream2; /** - * An alternate to {@link java.io.OutputStreamWriter} that supports + *

An alternate to {@link java.io.OutputStreamWriter} that supports * several optimized implementation for well known {@link Charset}s, - * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}. - * The implementations of this class will never buffer characters beyond a call to the + * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}.

+ *

The implementations of this class will never buffer characters or bytes beyond a call to the * {@link #write(char[], int, int)} method, thus written characters will always be available - * in converted form to the passed {@link OutputStream}. + * in converted form to the passed {@link OutputStream}

. */ -public abstract class AbstractOutputStreamWriter extends Writer +public abstract class WriteThroughWriter extends Writer { + static final int DEFAULT_MAX_WRITE_SIZE = 1024; private final int _maxWriteSize; private final char[] _chars; protected final OutputStream _out; protected final ByteArrayOutputStream2 _bytes; - protected AbstractOutputStreamWriter(OutputStream out) + protected WriteThroughWriter(OutputStream out) { this(out, 0); } @@ -47,9 +48,9 @@ protected AbstractOutputStreamWriter(OutputStream out) * @param out The {@link OutputStream} to write the converted bytes to. * @param maxWriteSize The maximum size in characters of a single conversion */ - protected AbstractOutputStreamWriter(OutputStream out, int maxWriteSize) + protected WriteThroughWriter(OutputStream out, int maxWriteSize) { - _maxWriteSize = maxWriteSize <= 0 ? 1024 : maxWriteSize; + _maxWriteSize = maxWriteSize <= 0 ? DEFAULT_MAX_WRITE_SIZE : maxWriteSize; _out = out; _chars = new char[_maxWriteSize]; _bytes = new ByteArrayOutputStream2(_maxWriteSize); @@ -63,7 +64,7 @@ protected AbstractOutputStreamWriter(OutputStream out, int maxWriteSize) * @return A Writer that will * @throws IOException If there is a problem creating the {@link Writer}. */ - public static Writer newWriter(OutputStream outputStream, String charset) + public static WriteThroughWriter newWriter(OutputStream outputStream, String charset) throws IOException { if (StandardCharsets.ISO_8859_1.name().equalsIgnoreCase(charset)) @@ -81,7 +82,7 @@ public static Writer newWriter(OutputStream outputStream, String charset) * @return A Writer that will * @throws IOException If there is a problem creating the {@link Writer}. */ - public static Writer newWriter(OutputStream outputStream, Charset charset) + public static WriteThroughWriter newWriter(OutputStream outputStream, Charset charset) throws IOException { if (StandardCharsets.ISO_8859_1 == charset) @@ -128,14 +129,14 @@ public void write(String string, int offset, int length) throws IOException public abstract void write(char[] s, int offset, int length) throws IOException; /** - * An implementation of {@link AbstractOutputStreamWriter} for + * An implementation of {@link WriteThroughWriter} for * optimal ISO-8859-1 conversion. * The ISO-8859-1 encoding is done by this class and no additional * buffers or Writers are used. */ - public static class Iso88591Writer extends AbstractOutputStreamWriter + private static class Iso88591Writer extends WriteThroughWriter { - public Iso88591Writer(OutputStream out) + private Iso88591Writer(OutputStream out) { super(out); } @@ -177,17 +178,17 @@ public void write(char[] s, int offset, int length) throws IOException } /** - * An implementation of {@link AbstractOutputStreamWriter} for + * An implementation of {@link WriteThroughWriter} for * an optimal UTF-8 conversion. * The UTF-8 encoding is done by this class and no additional * buffers or Writers are used. * The UTF-8 code was inspired by ... */ - public static class Utf8Writer extends AbstractOutputStreamWriter + private static class Utf8Writer extends WriteThroughWriter { int _surrogate = 0; - public Utf8Writer(OutputStream out) + private Utf8Writer(OutputStream out) { super(out); } @@ -334,10 +335,10 @@ else if ((code & 0x80000000) == 0) } /** - * An implementation of {@link AbstractOutputStreamWriter} that internally + * An implementation of {@link WriteThroughWriter} that internally * uses {@link java.io.OutputStreamWriter}. */ - public static class EncodingWriter extends AbstractOutputStreamWriter + private static class EncodingWriter extends WriteThroughWriter { final Writer _converter; diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java similarity index 85% rename from jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java rename to jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java index 08a091823051..572da1ee3fe8 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/writer/AbstractWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java @@ -11,14 +11,13 @@ // ======================================================================== // -package org.eclipse.jetty.io.writer; +package org.eclipse.jetty.io; import java.io.OutputStream; +import java.io.Writer; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import org.eclipse.jetty.io.AbstractOutputStreamWriter; -import org.eclipse.jetty.io.ByteBufferOutputStream; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Utf8StringBuilder; @@ -43,7 +42,7 @@ public void init() throws Exception @Test public void testSimpleUTF8() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); writer.write("Now is the time"); assertArrayEquals("Now is the time".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @@ -51,7 +50,7 @@ public void testSimpleUTF8() throws Exception @Test public void testUTF8() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); writer.write("How now Brown cow"); assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @@ -59,7 +58,7 @@ public void testUTF8() throws Exception @Test public void testUTF16() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.EncodingWriter(_out, StandardCharsets.UTF_16.displayName()); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_16); writer.write("How now Brown cow"); assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); } @@ -67,7 +66,7 @@ public void testUTF16() throws Exception @Test public void testNotCESU8() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); String data = "xxx𐐀xxx"; writer.write(data); byte[] b = BufferUtil.toArray(_bytes); @@ -83,8 +82,8 @@ public void testNotCESU8() throws Exception @Test public void testMultiByteOverflowUTF8() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); - int maxWriteSize = writer.getMaxWriteSize(); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); + int maxWriteSize = WriteThroughWriter.DEFAULT_MAX_WRITE_SIZE; final String singleByteStr = "a"; final String multiByteDuplicateStr = "B"; int remainSize = 1; @@ -107,7 +106,7 @@ public void testMultiByteOverflowUTF8() throws Exception @Test public void testISO8859() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Iso88591Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.ISO_8859_1); writer.write("How now Brown cow"); assertEquals(new String(BufferUtil.toArray(_bytes), StandardCharsets.ISO_8859_1), "How now ?rown cow"); } @@ -115,7 +114,7 @@ public void testISO8859() throws Exception @Test public void testUTF16x2() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); String source = "𠮟"; byte[] bytes = source.getBytes(StandardCharsets.UTF_8); @@ -137,7 +136,7 @@ public void testUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16x2() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); final String singleByteStr = "a"; int remainSize = 1; @@ -145,7 +144,7 @@ public void testMultiByteOverflowUTF16x2() throws Exception int adjustSize = -1; String source = - singleByteStr.repeat(Math.max(0, writer.getMaxWriteSize() + adjustSize)) + + singleByteStr.repeat(Math.max(0, WriteThroughWriter.DEFAULT_MAX_WRITE_SIZE + adjustSize)) + multiByteDuplicateStr + singleByteStr.repeat(remainSize); @@ -168,7 +167,7 @@ public void testMultiByteOverflowUTF16x2() throws Exception @Test public void testMultiByteOverflowUTF16X22() throws Exception { - AbstractOutputStreamWriter writer = new AbstractOutputStreamWriter.Utf8Writer(_out); + Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); final String singleByteStr = "a"; int remainSize = 1; @@ -176,7 +175,7 @@ public void testMultiByteOverflowUTF16X22() throws Exception int adjustSize = -2; String source = - singleByteStr.repeat(Math.max(0, writer.getMaxWriteSize() + adjustSize)) + + singleByteStr.repeat(Math.max(0, WriteThroughWriter.DEFAULT_MAX_WRITE_SIZE + adjustSize)) + multiByteDuplicateStr + singleByteStr.repeat(remainSize); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java index 927c7c844565..3e1ff04c71e5 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ResponseWriter.java @@ -16,7 +16,6 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.PrintWriter; -import java.io.Writer; import java.util.Formatter; import java.util.Locale; @@ -24,6 +23,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.WriteThroughWriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,14 +41,14 @@ public class ResponseWriter extends PrintWriter { private static final Logger LOG = LoggerFactory.getLogger(ResponseWriter.class); - private final Writer _writer; + private final WriteThroughWriter _writer; private final Locale _locale; private final String _encoding; private IOException _ioException; private boolean _isClosed = false; private Formatter _formatter; - public ResponseWriter(Writer writer, Locale locale, String encoding) + public ResponseWriter(WriteThroughWriter writer, Locale locale, String encoding) { super(writer, false); _writer = writer; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index ea3285ad882f..5cd5865b596c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -15,7 +15,6 @@ import java.io.IOException; import java.io.PrintWriter; -import java.io.Writer; import java.util.Collection; import java.util.EnumSet; import java.util.Locale; @@ -33,7 +32,7 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.io.AbstractOutputStreamWriter; +import org.eclipse.jetty.io.WriteThroughWriter; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.session.ManagedSession; @@ -317,7 +316,7 @@ public PrintWriter getWriter() throws IOException { // We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters // in the writer implementation for flush and completion operations. - Writer outputStreamWriter = AbstractOutputStreamWriter.newWriter(getServletChannel().getHttpOutput(), encoding); + WriteThroughWriter outputStreamWriter = WriteThroughWriter.newWriter(getServletChannel().getHttpOutput(), encoding); getServletResponseInfo().setWriter(writer = new ResponseWriter( outputStreamWriter, locale, encoding)); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index a710eb2840c2..57e19968fb6c 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -15,7 +15,6 @@ import java.io.IOException; import java.io.PrintWriter; -import java.io.Writer; import java.nio.channels.IllegalSelectorException; import java.util.Collection; import java.util.Collections; @@ -51,8 +50,8 @@ import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.http.PreEncodedHttpField; import org.eclipse.jetty.http.content.HttpContent; -import org.eclipse.jetty.io.AbstractOutputStreamWriter; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.WriteThroughWriter; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.HttpCookieUtils; import org.eclipse.jetty.server.HttpCookieUtils.SetCookieHttpField; @@ -876,7 +875,7 @@ public PrintWriter getWriter() throws IOException { // We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters // in the writer implementation for flush and completion operations. - Writer outputStreamWriter = AbstractOutputStreamWriter.newWriter(_out, encoding); + WriteThroughWriter outputStreamWriter = WriteThroughWriter.newWriter(_out, encoding); _writer = new ResponseWriter(outputStreamWriter, locale, encoding); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java index c7288d30a1d8..f0713a66cf69 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResponseWriter.java @@ -16,13 +16,13 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.PrintWriter; -import java.io.Writer; import java.util.Formatter; import java.util.Locale; import jakarta.servlet.ServletResponse; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.RuntimeIOException; +import org.eclipse.jetty.io.WriteThroughWriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,14 +40,14 @@ public class ResponseWriter extends PrintWriter { private static final Logger LOG = LoggerFactory.getLogger(ResponseWriter.class); - private final Writer _writer; + private final WriteThroughWriter _writer; private final Locale _locale; private final String _encoding; private IOException _ioException; private boolean _isClosed = false; private Formatter _formatter; - public ResponseWriter(Writer httpWriter, Locale locale, String encoding) + public ResponseWriter(WriteThroughWriter httpWriter, Locale locale, String encoding) { super(httpWriter, false); _writer = httpWriter; From 23efd4fea243c29e6f5e33a62af39fd06c905b01 Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 25 Sep 2023 10:13:53 +1000 Subject: [PATCH 07/15] Updates from review --- .../eclipse/jetty/io/WriteThroughWriter.java | 95 ++++++++++--------- ...rTest.java => WriteThroughWriterTest.java} | 4 +- .../org/eclipse/jetty/util/StringUtil.java | 64 +++++++++++++ .../eclipse/jetty/util/StringUtilTest.java | 44 +++++++++ 4 files changed, 158 insertions(+), 49 deletions(-) rename jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/{AbstractWriterTest.java => WriteThroughWriterTest.java} (98%) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java index 427961cc7c96..32cae875728a 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import org.eclipse.jetty.util.ByteArrayOutputStream2; +import org.eclipse.jetty.util.StringUtil; /** *

An alternate to {@link java.io.OutputStreamWriter} that supports @@ -34,7 +35,6 @@ public abstract class WriteThroughWriter extends Writer { static final int DEFAULT_MAX_WRITE_SIZE = 1024; private final int _maxWriteSize; - private final char[] _chars; protected final OutputStream _out; protected final ByteArrayOutputStream2 _bytes; @@ -52,7 +52,6 @@ protected WriteThroughWriter(OutputStream out, int maxWriteSize) { _maxWriteSize = maxWriteSize <= 0 ? DEFAULT_MAX_WRITE_SIZE : maxWriteSize; _out = out; - _chars = new char[_maxWriteSize]; _bytes = new ByteArrayOutputStream2(_maxWriteSize); } @@ -109,24 +108,34 @@ public void flush() throws IOException _out.flush(); } + @Override + public abstract WriteThroughWriter append(CharSequence sequence) throws IOException; + @Override public void write(String string, int offset, int length) throws IOException { while (length > _maxWriteSize) { - int end = offset + _maxWriteSize; - string.getChars(offset, end, _chars, 0); - write(_chars, 0, length); - offset = end; + append(StringUtil.subSequence(string, offset, _maxWriteSize)); + offset += _maxWriteSize; length -= _maxWriteSize; } - string.getChars(offset, offset + length, _chars, 0); - write(_chars, 0, length); + append(StringUtil.subSequence(string, offset, length)); } @Override - public abstract void write(char[] s, int offset, int length) throws IOException; + public void write(char[] chars, int offset, int length) throws IOException + { + while (length > _maxWriteSize) + { + append(StringUtil.subSequence(chars, offset, _maxWriteSize)); + offset += _maxWriteSize; + length -= _maxWriteSize; + } + + append(StringUtil.subSequence(chars, offset, length)); + } /** * An implementation of {@link WriteThroughWriter} for @@ -142,38 +151,30 @@ private Iso88591Writer(OutputStream out) } @Override - public void write(char[] s, int offset, int length) throws IOException + public WriteThroughWriter append(CharSequence charSequence) throws IOException { - if (length == 1) + assert charSequence.length() <= getMaxWriteSize(); + + if (charSequence.length() == 1) { - int c = s[offset]; + int c = charSequence.charAt(0); _out.write(c < 256 ? c : '?'); - return; + return this; } - while (length > 0) + _bytes.reset(); + int bytes = 0; + byte[] buffer = _bytes.getBuf(); + int length = charSequence.length(); + for (int offset = 0; offset < length; offset++) { - _bytes.reset(); - int chars = Math.min(length, getMaxWriteSize()); - - byte[] buffer = _bytes.getBuf(); - int bytes = _bytes.getCount(); - - if (chars > buffer.length - bytes) - chars = buffer.length - bytes; - - for (int i = 0; i < chars; i++) - { - int c = s[offset + i]; - buffer[bytes++] = (byte)(c < 256 ? c : '?'); - } - if (bytes >= 0) - _bytes.setCount(bytes); - - _bytes.writeTo(_out); - length -= chars; - offset += chars; + int c = charSequence.charAt(offset); + buffer[bytes++] = (byte)(c < 256 ? c : '?'); } + if (bytes >= 0) + _bytes.setCount(bytes); + _bytes.writeTo(_out); + return this; } } @@ -194,8 +195,11 @@ private Utf8Writer(OutputStream out) } @Override - public void write(char[] s, int offset, int length) throws IOException + public WriteThroughWriter append(CharSequence charSequence) throws IOException { + assert charSequence.length() <= getMaxWriteSize(); + int length = charSequence.length(); + int offset = 0; while (length > 0) { _bytes.reset(); @@ -209,7 +213,7 @@ public void write(char[] s, int offset, int length) throws IOException for (int i = 0; i < chars; i++) { - int code = s[offset + i]; + int code = charSequence.charAt(offset + i); // Do we already have a surrogate? if (_surrogate == 0) @@ -331,6 +335,7 @@ else if ((code & 0x80000000) == 0) length -= chars; offset += chars; } + return this; } } @@ -355,19 +360,15 @@ public EncodingWriter(OutputStream out, Charset charset) throws IOException } @Override - public void write(char[] s, int offset, int length) throws IOException + public WriteThroughWriter append(CharSequence charSequence) throws IOException { - while (length > 0) - { - _bytes.reset(); - int chars = Math.min(length, getMaxWriteSize()); + assert charSequence.length() <= getMaxWriteSize(); - _converter.write(s, offset, chars); - _converter.flush(); - _bytes.writeTo(_out); - length -= chars; - offset += chars; - } + _bytes.reset(); + _converter.append(charSequence); + _converter.flush(); + _bytes.writeTo(_out); + return this; } } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java similarity index 98% rename from jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java rename to jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java index 572da1ee3fe8..fe2357e43406 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/AbstractWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java @@ -27,7 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; -public class AbstractWriterTest +public class WriteThroughWriterTest { private OutputStream _out; private ByteBuffer _bytes; @@ -197,7 +197,7 @@ public void testMultiByteOverflowUTF16X22() throws Exception private void myReportBytes(byte[] bytes) throws Exception { - if (LoggerFactory.getLogger(AbstractWriterTest.class).isDebugEnabled()) + if (LoggerFactory.getLogger(WriteThroughWriterTest.class).isDebugEnabled()) { for (int i = 0; i < bytes.length; i++) System.err.format("%s%x", (i == 0) ? "[" : (i % 512 == 0) ? "][" : ",", bytes[i]); diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index a8c106b919a3..c3f01de0d54f 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.util; import java.io.UnsupportedEncodingException; +import java.nio.CharBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -480,6 +481,69 @@ public static boolean equals(String s, char[] buf, int offset, int length) return true; } + /** + * Get a zero copy subsequence of a {@link String}. + * @param string The {@link String} to take a subsequence of. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence, backed by the passed {@link String} + * or the original {@link String} if it is the same. + */ + public static CharSequence subSequence(String string, int offset, int length) + { + Objects.requireNonNull(string); + + if (offset == 0 && string.length() == length) + return string; + if (length == 0) + return ""; + + if (offset < 0 || length < 0 || (offset + length) > string.length()) + throw new IndexOutOfBoundsException("offset and/or length out of range"); + + return new CharSequence() + { + @Override + public int length() + { + return length; + } + + @Override + public char charAt(int index) + { + return string.charAt(offset + index); + } + + @Override + public CharSequence subSequence(int start, int end) + { + return StringUtil.subSequence(string, offset + start, end - start); + } + + @Override + public String toString() + { + return string.substring(offset, offset + length); + } + }; + } + + /** + * Get a zero copy subsequence of a {@code char} array. + * @param chars The characters to take a subsequence of. These character are not copied and the array should not be + * modified for the life of the returned CharSequence. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence. + */ + public static CharSequence subSequence(char[] chars, int offset, int length) + { + if (length == 0) + return ""; + return CharBuffer.wrap(chars, offset, length); + } + public static String toUTF8String(byte[] b, int offset, int length) { return new String(b, offset, length, StandardCharsets.UTF_8); diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java index c970f269a5dd..6c78b9c3e064 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java @@ -23,6 +23,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.emptyArray; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -35,6 +36,7 @@ // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck public class StringUtilTest { + @Test @SuppressWarnings("ReferenceEquality") public void testAsciiToLowerCase() @@ -294,4 +296,46 @@ public void testToHexStringEmpty() { assertThat(StringUtil.toHexString(new byte[0]), is("")); } + + public static Stream subSequenceTests() + { + return Stream.of( + Arguments.of("", 0, 0, ""), + Arguments.of("", 0, 1, null), + Arguments.of("", 1, 0, ""), + Arguments.of("", 1, 1, null), + Arguments.of("hello", 0, 5, "hello"), + Arguments.of("hello", 0, 4, "hell"), + Arguments.of("hello", 1, 4, "ello"), + Arguments.of("hello", 1, 3, "ell"), + Arguments.of("hello", 5, 0, ""), + Arguments.of("hello", 0, 6, null) + ); + } + + @ParameterizedTest + @MethodSource("subSequenceTests") + public void testSubSequence(String source, int offset, int length, String expected) + { + if (expected == null) + { + assertThrows(IndexOutOfBoundsException.class, () -> StringUtil.subSequence(source, offset, length)); + assertThrows(IndexOutOfBoundsException.class, () -> StringUtil.subSequence(source.toCharArray(), offset, length)); + return; + } + + CharSequence result = StringUtil.subSequence(source, offset, length); + assertThat(result.toString(), equalTo(expected)); + + // check string optimization + if (offset == 0 && length == source.length()) + { + assertThat(result, sameInstance(source)); + assertThat(result.subSequence(offset, length), sameInstance(source)); + return; + } + + result = StringUtil.subSequence(source.toCharArray(), offset, length); + assertThat(result.toString(), equalTo(expected)); + } } From b5f069980ed7b66025bed5456ba222d67fd1a972 Mon Sep 17 00:00:00 2001 From: gregw Date: Mon, 25 Sep 2023 10:28:59 +1000 Subject: [PATCH 08/15] Updates from review --- .../src/main/java/org/eclipse/jetty/util/StringUtil.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index c3f01de0d54f..42f3ddd75ff9 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -498,7 +498,8 @@ public static CharSequence subSequence(String string, int offset, int length) if (length == 0) return ""; - if (offset < 0 || length < 0 || (offset + length) > string.length()) + int end = offset + length; + if (offset < 0 || offset > end || end > string.length()) throw new IndexOutOfBoundsException("offset and/or length out of range"); return new CharSequence() @@ -539,6 +540,7 @@ public String toString() */ public static CharSequence subSequence(char[] chars, int offset, int length) { + // Needed to make bounds check of wrap the same as for string.substring if (length == 0) return ""; return CharBuffer.wrap(chars, offset, length); From aaceef05149161d115ec9a394138c8b142558306 Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 26 Sep 2023 10:58:52 +1000 Subject: [PATCH 09/15] WIP --- .../org/eclipse/jetty/io/QuietException.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java index 2340e76372a2..b1ac44fe9119 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java @@ -53,4 +53,26 @@ public Exception(Throwable cause) super(cause); } } + + class Runtime extends RuntimeException implements QuietException + { + public Runtime() + { + } + + public Runtime(String message) + { + super(message); + } + + public Runtime(String message, Throwable cause) + { + super(message, cause); + } + + public Runtime(Throwable cause) + { + super(cause); + } + } } From 676799cd9f4c2cbb83836b81fcabb4822cb7ec18 Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 26 Sep 2023 19:40:15 +1000 Subject: [PATCH 10/15] Added flush mechanism --- .../jetty/io/ByteBufferAggregator.java | 9 ++ .../jetty/io/content/BufferedContentSink.java | 86 +++++++++++++------ .../io/content/ContentSinkOutputStream.java | 9 +- .../jetty/io/BufferedContentSinkTest.java | 45 ++++++++++ 4 files changed, 123 insertions(+), 26 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java index 3d929d2d5f20..5670d08dc603 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java @@ -58,6 +58,15 @@ public ByteBufferAggregator(ByteBufferPool bufferPool, boolean direct, int start _currentSize = startSize; } + /** + * Get the currently aggregated length. + * @return The current total aggregated bytes. + */ + public int length() + { + return _aggregatedSize; + } + /** * Aggregates the given ByteBuffer. This copies bytes up to the specified maximum size, at which * time this method returns {@code true} and {@link #takeRetainableByteBuffer()} must be called diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java index d835716e7b15..b58b17850131 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java @@ -35,6 +35,19 @@ */ public class BufferedContentSink implements Content.Sink { + /** + * An empty byte array, which if {@link #write(boolean, ByteBuffer, Callback) written} + * as a {@link ByteBuffer#wrap(byte[]) wrapped ByteBuffer} + * will invoke a {@link #flush(Callback)} operation. + */ + public static final byte[] FLUSH_BYTES = new byte[0]; + + /** + * An empty {@link ByteBuffer}, which if {@link #write(boolean, ByteBuffer, Callback) written} + * will invoke a {@link #flush(Callback)} operation. + */ + public static final ByteBuffer FLUSH_BUFFER = ByteBuffer.wrap(FLUSH_BYTES); + private static final Logger LOG = LoggerFactory.getLogger(BufferedContentSink.class); private static final int START_BUFFER_SIZE = 1024; @@ -103,6 +116,15 @@ public void write(boolean last, ByteBuffer byteBuffer, Callback callback) } } + /** + * Flush the buffered content. + * @param callback Callback completed when the flush is complete + */ + public void flush(Callback callback) + { + flush(false, FLUSH_BUFFER, callback); + } + /** * Flushes the aggregated buffer if something was aggregated, then flushes the * given buffer, bypassing the aggregator. @@ -119,7 +141,7 @@ private void flush(boolean last, ByteBuffer currentBuffer, Callback callback) LOG.debug("nothing aggregated, flushing current buffer {}", currentBuffer); _flusher.offer(last, currentBuffer, callback); } - else + else if (BufferUtil.hasContent(currentBuffer)) { if (LOG.isDebugEnabled()) LOG.debug("flushing aggregated buffer {}", aggregatedBuffer); @@ -144,6 +166,10 @@ public void failed(Throwable x) } }); } + else + { + _flusher.offer(false, aggregatedBuffer.getByteBuffer(), Callback.from(aggregatedBuffer::release, callback)); + } } /** @@ -152,7 +178,11 @@ public void failed(Throwable x) private void aggregateAndFlush(boolean last, ByteBuffer currentBuffer, Callback callback) { boolean full = _aggregator.aggregate(currentBuffer); - boolean complete = last && !currentBuffer.hasRemaining(); + boolean empty = !currentBuffer.hasRemaining(); + boolean flush = full || + currentBuffer == FLUSH_BUFFER || + empty && currentBuffer.hasArray() && currentBuffer.array() == FLUSH_BYTES; + boolean complete = last && empty; if (LOG.isDebugEnabled()) LOG.debug("aggregated current buffer, full={}, complete={}, bytes left={}, aggregator={}", full, complete, currentBuffer.remaining(), _aggregator); if (complete) @@ -171,34 +201,42 @@ private void aggregateAndFlush(boolean last, ByteBuffer currentBuffer, Callback _flusher.offer(true, BufferUtil.EMPTY_BUFFER, callback); } } - else if (full) + else if (flush) { RetainableByteBuffer aggregatedBuffer = _aggregator.takeRetainableByteBuffer(); if (LOG.isDebugEnabled()) - LOG.debug("writing aggregated buffer: {} bytes", aggregatedBuffer.remaining()); - _flusher.offer(false, aggregatedBuffer.getByteBuffer(), new Callback.Nested(Callback.from(aggregatedBuffer::release)) + LOG.debug("writing aggregated buffer: {} bytes, then {}", aggregatedBuffer.remaining(), currentBuffer.remaining()); + + if (BufferUtil.hasContent(currentBuffer)) { - @Override - public void succeeded() + _flusher.offer(false, aggregatedBuffer.getByteBuffer(), new Callback.Nested(Callback.from(aggregatedBuffer::release)) { - super.succeeded(); - if (LOG.isDebugEnabled()) - LOG.debug("written aggregated buffer, writing remaining of current: {} bytes{}", currentBuffer.remaining(), (last ? " (last write)" : "")); - if (last) - _flusher.offer(true, currentBuffer, callback); - else - aggregateAndFlush(false, currentBuffer, callback); - } + @Override + public void succeeded() + { + super.succeeded(); + if (LOG.isDebugEnabled()) + LOG.debug("written aggregated buffer, writing remaining of current: {} bytes{}", currentBuffer.remaining(), (last ? " (last write)" : "")); + if (last) + _flusher.offer(true, currentBuffer, callback); + else + aggregateAndFlush(false, currentBuffer, callback); + } - @Override - public void failed(Throwable x) - { - if (LOG.isDebugEnabled()) - LOG.debug("failure writing aggregated buffer", x); - super.failed(x); - callback.failed(x); - } - }); + @Override + public void failed(Throwable x) + { + if (LOG.isDebugEnabled()) + LOG.debug("failure writing aggregated buffer", x); + super.failed(x); + callback.failed(x); + } + }); + } + else + { + _flusher.offer(false, aggregatedBuffer.getByteBuffer(), Callback.from(aggregatedBuffer::release, callback)); + } } else { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSinkOutputStream.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSinkOutputStream.java index 7484681f005f..c02498fa2a5a 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSinkOutputStream.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSinkOutputStream.java @@ -64,7 +64,7 @@ public void flush() throws IOException { try (Blocker.Callback callback = _blocking.callback()) { - sink.write(false, null, callback); + sink.write(false, BufferedContentSink.FLUSH_BUFFER, callback); callback.block(); } catch (Throwable x) @@ -78,7 +78,7 @@ public void close() throws IOException { try (Blocker.Callback callback = _blocking.callback()) { - sink.write(true, null, callback); + close(callback); callback.block(); } catch (Throwable x) @@ -86,4 +86,9 @@ public void close() throws IOException throw IO.rethrow(x); } } + + public void close(Callback callback) throws IOException + { + sink.write(true, null, callback); + } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java index 2d1b2bbaf817..314e5c0031ba 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java @@ -21,6 +21,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiConsumer; +import java.util.stream.Stream; import org.eclipse.jetty.io.content.AsyncContent; import org.eclipse.jetty.io.content.BufferedContentSink; @@ -29,6 +31,8 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import static java.nio.charset.StandardCharsets.UTF_8; import static org.awaitility.Awaitility.await; @@ -39,6 +43,7 @@ import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -229,6 +234,46 @@ public void testSmallBuffer() } } + public static Stream> flushers() + { + return Stream.of( + BufferedContentSink::flush, + (b, callback) -> b.write(false, BufferedContentSink.FLUSH_BUFFER, callback), + (b, callback) -> b.write(false, ByteBuffer.wrap(BufferedContentSink.FLUSH_BYTES), callback) + ); + } + + @ParameterizedTest + @MethodSource("flushers") + public void testFlush(BiConsumer flusher) throws Exception + { + ByteBuffer accumulatingBuffer = BufferUtil.allocate(4096); + BufferUtil.flipToFill(accumulatingBuffer); + + try (AsyncContent async = new AsyncContent(); ) + { + BufferedContentSink buffered = new BufferedContentSink(async, _bufferPool, false, 8192, 8192); + + Callback.Completable callback = new Callback.Completable(); + buffered.write(false, BufferUtil.toBuffer("Hello "), callback); + callback.get(5, TimeUnit.SECONDS); + assertNull(async.read()); + + callback = new Callback.Completable(); + buffered.write(false, BufferUtil.toBuffer("World!"), callback); + callback.get(5, TimeUnit.SECONDS); + assertNull(async.read()); + + callback = new Callback.Completable(); + flusher.accept(buffered, callback); + Content.Chunk chunk = async.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer()), is("Hello World!")); + chunk.release(); + callback.get(5, TimeUnit.SECONDS); + } + } + @Test public void testMaxAggregationSizeExceeded() { From 59102052fe06e17733ace24a497574b0b50f95d3 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 27 Sep 2023 06:37:54 +1000 Subject: [PATCH 11/15] Updates from review --- .../java/org/eclipse/jetty/io/ChunkAccumulator.java | 2 ++ .../java/org/eclipse/jetty/io/QuietException.java | 10 +++++----- .../org/eclipse/jetty/io/WriteThroughWriter.java | 4 ++-- .../jetty/io/content/BufferedContentSink.java | 13 ++----------- .../eclipse/jetty/io/BufferedContentSinkTest.java | 3 +-- .../java/org/eclipse/jetty/util/StringUtil.java | 6 +++++- .../java/org/eclipse/jetty/ee9/nested/Response.java | 2 +- 7 files changed, 18 insertions(+), 22 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java index bf9fb6ea1450..cb036274c09a 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -71,6 +71,8 @@ public int length() public byte[] take() { + if (_length == 0) + return BufferUtil.EMPTY_BUFFER.array(); byte[] bytes = new byte[_length]; int offset = 0; for (Chunk chunk : _chunks) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java index b1ac44fe9119..c51f50eed113 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/QuietException.java @@ -54,23 +54,23 @@ public Exception(Throwable cause) } } - class Runtime extends RuntimeException implements QuietException + class RuntimeException extends java.lang.RuntimeException implements QuietException { - public Runtime() + public RuntimeException() { } - public Runtime(String message) + public RuntimeException(String message) { super(message); } - public Runtime(String message, Throwable cause) + public RuntimeException(String message, Throwable cause) { super(message, cause); } - public Runtime(Throwable cause) + public RuntimeException(Throwable cause) { super(cause); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java index 32cae875728a..0c4188ced5b2 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java @@ -28,8 +28,8 @@ * several optimized implementation for well known {@link Charset}s, * specifically {@link StandardCharsets#UTF_8} and {@link StandardCharsets#ISO_8859_1}.

*

The implementations of this class will never buffer characters or bytes beyond a call to the - * {@link #write(char[], int, int)} method, thus written characters will always be available - * in converted form to the passed {@link OutputStream}

. + * {@link #write(char[], int, int)} method, thus written characters will always be passed + * as bytes to the passed {@link OutputStream}

. */ public abstract class WriteThroughWriter extends Writer { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java index b58b17850131..acb85e2ddb60 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java @@ -35,18 +35,11 @@ */ public class BufferedContentSink implements Content.Sink { - /** - * An empty byte array, which if {@link #write(boolean, ByteBuffer, Callback) written} - * as a {@link ByteBuffer#wrap(byte[]) wrapped ByteBuffer} - * will invoke a {@link #flush(Callback)} operation. - */ - public static final byte[] FLUSH_BYTES = new byte[0]; - /** * An empty {@link ByteBuffer}, which if {@link #write(boolean, ByteBuffer, Callback) written} * will invoke a {@link #flush(Callback)} operation. */ - public static final ByteBuffer FLUSH_BUFFER = ByteBuffer.wrap(FLUSH_BYTES); + public static final ByteBuffer FLUSH_BUFFER = ByteBuffer.wrap(new byte[0]); private static final Logger LOG = LoggerFactory.getLogger(BufferedContentSink.class); @@ -179,9 +172,7 @@ private void aggregateAndFlush(boolean last, ByteBuffer currentBuffer, Callback { boolean full = _aggregator.aggregate(currentBuffer); boolean empty = !currentBuffer.hasRemaining(); - boolean flush = full || - currentBuffer == FLUSH_BUFFER || - empty && currentBuffer.hasArray() && currentBuffer.array() == FLUSH_BYTES; + boolean flush = full || currentBuffer == FLUSH_BUFFER; boolean complete = last && empty; if (LOG.isDebugEnabled()) LOG.debug("aggregated current buffer, full={}, complete={}, bytes left={}, aggregator={}", full, complete, currentBuffer.remaining(), _aggregator); diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java index 314e5c0031ba..0731eff5d32b 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java @@ -238,8 +238,7 @@ public static Stream> flushers() { return Stream.of( BufferedContentSink::flush, - (b, callback) -> b.write(false, BufferedContentSink.FLUSH_BUFFER, callback), - (b, callback) -> b.write(false, ByteBuffer.wrap(BufferedContentSink.FLUSH_BYTES), callback) + (b, callback) -> b.write(false, BufferedContentSink.FLUSH_BUFFER, callback) ); } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index 42f3ddd75ff9..ef46033073c3 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -482,7 +482,11 @@ public static boolean equals(String s, char[] buf, int offset, int length) } /** - * Get a zero copy subsequence of a {@link String}. + *

Get a zero copy subsequence of a {@link String}.

+ *

Use of this is method can result in unforeseen GC consequences and can bypass + * JVM optimizations available in {@link String#subSequence(int, int)}. It should only + * be used in cases where there is a known benefit: large sub sequence of a larger string with no retained + * references to the sub sequence beyond the life time of the string.

* @param string The {@link String} to take a subsequence of. * @param offset The offset in characters into the string to start the subsequence * @param length The length in characters of the substring diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java index 57e19968fb6c..e6611b5ce269 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Response.java @@ -873,7 +873,7 @@ public PrintWriter getWriter() throws IOException } else { - // We must use an implementation of AbstractOutputStreamWriter here as we rely on the non cached characters + // We must use an specialized Writer here as we rely on the non cached characters // in the writer implementation for flush and completion operations. WriteThroughWriter outputStreamWriter = WriteThroughWriter.newWriter(_out, encoding); _writer = new ResponseWriter(outputStreamWriter, locale, encoding); From 25be1ca17a5757888f1dec3dd2981f3f8276e874 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 27 Sep 2023 06:42:37 +1000 Subject: [PATCH 12/15] Updates from review --- .../eclipse/jetty/io/WriteThroughWriter.java | 80 +++++++++++++++++-- .../jetty/io/WriteThroughWriterTest.java | 50 ++++++++++++ .../org/eclipse/jetty/util/StringUtil.java | 70 ---------------- .../eclipse/jetty/util/StringUtilTest.java | 43 ---------- 4 files changed, 125 insertions(+), 118 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java index 0c4188ced5b2..221a8e2e6e31 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java @@ -17,11 +17,12 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Writer; +import java.nio.CharBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.util.Objects; import org.eclipse.jetty.util.ByteArrayOutputStream2; -import org.eclipse.jetty.util.StringUtil; /** *

An alternate to {@link java.io.OutputStreamWriter} that supports @@ -91,6 +92,75 @@ public static WriteThroughWriter newWriter(OutputStream outputStream, Charset ch return new EncodingWriter(outputStream, charset); } + /** + *

Get a zero copy subsequence of a {@link String}.

+ *

Use of this is method can result in unforeseen GC consequences and can bypass + * JVM optimizations available in {@link String#subSequence(int, int)}. It should only + * be used in cases where there is a known benefit: large sub sequence of a larger string with no retained + * references to the sub sequence beyond the life time of the string.

+ * @param string The {@link String} to take a subsequence of. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence, backed by the passed {@link String} + * or the original {@link String} if it is the same. + */ + static CharSequence subSequence(String string, int offset, int length) + { + Objects.requireNonNull(string); + + if (offset == 0 && string.length() == length) + return string; + if (length == 0) + return ""; + + int end = offset + length; + if (offset < 0 || offset > end || end > string.length()) + throw new IndexOutOfBoundsException("offset and/or length out of range"); + + return new CharSequence() + { + @Override + public int length() + { + return length; + } + + @Override + public char charAt(int index) + { + return string.charAt(offset + index); + } + + @Override + public CharSequence subSequence(int start, int end) + { + return WriteThroughWriter.subSequence(string, offset + start, end - start); + } + + @Override + public String toString() + { + return string.substring(offset, offset + length); + } + }; + } + + /** + * Get a zero copy subsequence of a {@code char} array. + * @param chars The characters to take a subsequence of. These character are not copied and the array should not be + * modified for the life of the returned CharSequence. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence. + */ + static CharSequence subSequence(char[] chars, int offset, int length) + { + // Needed to make bounds check of wrap the same as for string.substring + if (length == 0) + return ""; + return CharBuffer.wrap(chars, offset, length); + } + public int getMaxWriteSize() { return _maxWriteSize; @@ -116,12 +186,12 @@ public void write(String string, int offset, int length) throws IOException { while (length > _maxWriteSize) { - append(StringUtil.subSequence(string, offset, _maxWriteSize)); + append(subSequence(string, offset, _maxWriteSize)); offset += _maxWriteSize; length -= _maxWriteSize; } - append(StringUtil.subSequence(string, offset, length)); + append(subSequence(string, offset, length)); } @Override @@ -129,12 +199,12 @@ public void write(char[] chars, int offset, int length) throws IOException { while (length > _maxWriteSize) { - append(StringUtil.subSequence(chars, offset, _maxWriteSize)); + append(subSequence(chars, offset, _maxWriteSize)); offset += _maxWriteSize; length -= _maxWriteSize; } - append(StringUtil.subSequence(chars, offset, length)); + append(subSequence(chars, offset, length)); } /** diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java index 97d7d047c937..0c4558f33205 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java @@ -17,15 +17,23 @@ import java.io.Writer; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.stream.Stream; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Utf8StringBuilder; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.slf4j.LoggerFactory; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; public class WriteThroughWriterTest { @@ -214,4 +222,46 @@ private void assertArrayEquals(byte[] b1, byte[] b2) assertEquals(b1[i], b2[i], test); } } + + public static Stream subSequenceTests() + { + return Stream.of( + Arguments.of("", 0, 0, ""), + Arguments.of("", 0, 1, null), + Arguments.of("", 1, 0, ""), + Arguments.of("", 1, 1, null), + Arguments.of("hello", 0, 5, "hello"), + Arguments.of("hello", 0, 4, "hell"), + Arguments.of("hello", 1, 4, "ello"), + Arguments.of("hello", 1, 3, "ell"), + Arguments.of("hello", 5, 0, ""), + Arguments.of("hello", 0, 6, null) + ); + } + + @ParameterizedTest + @MethodSource("subSequenceTests") + public void testSubSequence(String source, int offset, int length, String expected) + { + if (expected == null) + { + assertThrows(IndexOutOfBoundsException.class, () -> WriteThroughWriter.subSequence(source, offset, length)); + assertThrows(IndexOutOfBoundsException.class, () -> WriteThroughWriter.subSequence(source.toCharArray(), offset, length)); + return; + } + + CharSequence result = WriteThroughWriter.subSequence(source, offset, length); + assertThat(result.toString(), equalTo(expected)); + + // check string optimization + if (offset == 0 && length == source.length()) + { + assertThat(result, sameInstance(source)); + assertThat(result.subSequence(offset, length), sameInstance(source)); + return; + } + + result = WriteThroughWriter.subSequence(source.toCharArray(), offset, length); + assertThat(result.toString(), equalTo(expected)); + } } diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index ef46033073c3..a8c106b919a3 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.util; import java.io.UnsupportedEncodingException; -import java.nio.CharBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.ArrayList; @@ -481,75 +480,6 @@ public static boolean equals(String s, char[] buf, int offset, int length) return true; } - /** - *

Get a zero copy subsequence of a {@link String}.

- *

Use of this is method can result in unforeseen GC consequences and can bypass - * JVM optimizations available in {@link String#subSequence(int, int)}. It should only - * be used in cases where there is a known benefit: large sub sequence of a larger string with no retained - * references to the sub sequence beyond the life time of the string.

- * @param string The {@link String} to take a subsequence of. - * @param offset The offset in characters into the string to start the subsequence - * @param length The length in characters of the substring - * @return A new {@link CharSequence} containing the subsequence, backed by the passed {@link String} - * or the original {@link String} if it is the same. - */ - public static CharSequence subSequence(String string, int offset, int length) - { - Objects.requireNonNull(string); - - if (offset == 0 && string.length() == length) - return string; - if (length == 0) - return ""; - - int end = offset + length; - if (offset < 0 || offset > end || end > string.length()) - throw new IndexOutOfBoundsException("offset and/or length out of range"); - - return new CharSequence() - { - @Override - public int length() - { - return length; - } - - @Override - public char charAt(int index) - { - return string.charAt(offset + index); - } - - @Override - public CharSequence subSequence(int start, int end) - { - return StringUtil.subSequence(string, offset + start, end - start); - } - - @Override - public String toString() - { - return string.substring(offset, offset + length); - } - }; - } - - /** - * Get a zero copy subsequence of a {@code char} array. - * @param chars The characters to take a subsequence of. These character are not copied and the array should not be - * modified for the life of the returned CharSequence. - * @param offset The offset in characters into the string to start the subsequence - * @param length The length in characters of the substring - * @return A new {@link CharSequence} containing the subsequence. - */ - public static CharSequence subSequence(char[] chars, int offset, int length) - { - // Needed to make bounds check of wrap the same as for string.substring - if (length == 0) - return ""; - return CharBuffer.wrap(chars, offset, length); - } - public static String toUTF8String(byte[] b, int offset, int length) { return new String(b, offset, length, StandardCharsets.UTF_8); diff --git a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java index 6c78b9c3e064..c04ca5aa0c36 100644 --- a/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java +++ b/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/StringUtilTest.java @@ -23,7 +23,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.emptyArray; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -296,46 +295,4 @@ public void testToHexStringEmpty() { assertThat(StringUtil.toHexString(new byte[0]), is("")); } - - public static Stream subSequenceTests() - { - return Stream.of( - Arguments.of("", 0, 0, ""), - Arguments.of("", 0, 1, null), - Arguments.of("", 1, 0, ""), - Arguments.of("", 1, 1, null), - Arguments.of("hello", 0, 5, "hello"), - Arguments.of("hello", 0, 4, "hell"), - Arguments.of("hello", 1, 4, "ello"), - Arguments.of("hello", 1, 3, "ell"), - Arguments.of("hello", 5, 0, ""), - Arguments.of("hello", 0, 6, null) - ); - } - - @ParameterizedTest - @MethodSource("subSequenceTests") - public void testSubSequence(String source, int offset, int length, String expected) - { - if (expected == null) - { - assertThrows(IndexOutOfBoundsException.class, () -> StringUtil.subSequence(source, offset, length)); - assertThrows(IndexOutOfBoundsException.class, () -> StringUtil.subSequence(source.toCharArray(), offset, length)); - return; - } - - CharSequence result = StringUtil.subSequence(source, offset, length); - assertThat(result.toString(), equalTo(expected)); - - // check string optimization - if (offset == 0 && length == source.length()) - { - assertThat(result, sameInstance(source)); - assertThat(result.subSequence(offset, length), sameInstance(source)); - return; - } - - result = StringUtil.subSequence(source.toCharArray(), offset, length); - assertThat(result.toString(), equalTo(expected)); - } } From b02e6831a8faee8ce50dd9a0743bf5a31301451d Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 27 Sep 2023 07:12:22 +1000 Subject: [PATCH 13/15] Updates from review --- .../eclipse/jetty/io/WriteThroughWriter.java | 142 +++++++++--------- .../jetty/io/WriteThroughWriterTest.java | 20 +-- 2 files changed, 81 insertions(+), 81 deletions(-) diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java index 221a8e2e6e31..362f1104ed87 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/WriteThroughWriter.java @@ -36,8 +36,8 @@ public abstract class WriteThroughWriter extends Writer { static final int DEFAULT_MAX_WRITE_SIZE = 1024; private final int _maxWriteSize; - protected final OutputStream _out; - protected final ByteArrayOutputStream2 _bytes; + final OutputStream _out; + final ByteArrayOutputStream2 _bytes; protected WriteThroughWriter(OutputStream out) { @@ -92,75 +92,6 @@ public static WriteThroughWriter newWriter(OutputStream outputStream, Charset ch return new EncodingWriter(outputStream, charset); } - /** - *

Get a zero copy subsequence of a {@link String}.

- *

Use of this is method can result in unforeseen GC consequences and can bypass - * JVM optimizations available in {@link String#subSequence(int, int)}. It should only - * be used in cases where there is a known benefit: large sub sequence of a larger string with no retained - * references to the sub sequence beyond the life time of the string.

- * @param string The {@link String} to take a subsequence of. - * @param offset The offset in characters into the string to start the subsequence - * @param length The length in characters of the substring - * @return A new {@link CharSequence} containing the subsequence, backed by the passed {@link String} - * or the original {@link String} if it is the same. - */ - static CharSequence subSequence(String string, int offset, int length) - { - Objects.requireNonNull(string); - - if (offset == 0 && string.length() == length) - return string; - if (length == 0) - return ""; - - int end = offset + length; - if (offset < 0 || offset > end || end > string.length()) - throw new IndexOutOfBoundsException("offset and/or length out of range"); - - return new CharSequence() - { - @Override - public int length() - { - return length; - } - - @Override - public char charAt(int index) - { - return string.charAt(offset + index); - } - - @Override - public CharSequence subSequence(int start, int end) - { - return WriteThroughWriter.subSequence(string, offset + start, end - start); - } - - @Override - public String toString() - { - return string.substring(offset, offset + length); - } - }; - } - - /** - * Get a zero copy subsequence of a {@code char} array. - * @param chars The characters to take a subsequence of. These character are not copied and the array should not be - * modified for the life of the returned CharSequence. - * @param offset The offset in characters into the string to start the subsequence - * @param length The length in characters of the substring - * @return A new {@link CharSequence} containing the subsequence. - */ - static CharSequence subSequence(char[] chars, int offset, int length) - { - // Needed to make bounds check of wrap the same as for string.substring - if (length == 0) - return ""; - return CharBuffer.wrap(chars, offset, length); - } - public int getMaxWriteSize() { return _maxWriteSize; @@ -441,4 +372,73 @@ public WriteThroughWriter append(CharSequence charSequence) throws IOException return this; } } + + /** + *

Get a zero copy subsequence of a {@link String}.

+ *

Use of this is method can result in unforeseen GC consequences and can bypass + * JVM optimizations available in {@link String#subSequence(int, int)}. It should only + * be used in cases where there is a known benefit: large sub sequence of a larger string with no retained + * references to the sub sequence beyond the life time of the string.

+ * @param string The {@link String} to take a subsequence of. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence, backed by the passed {@link String} + * or the original {@link String} if it is the same. + */ + static CharSequence subSequence(String string, int offset, int length) + { + Objects.requireNonNull(string); + + if (offset == 0 && string.length() == length) + return string; + if (length == 0) + return ""; + + int end = offset + length; + if (offset < 0 || offset > end || end > string.length()) + throw new IndexOutOfBoundsException("offset and/or length out of range"); + + return new CharSequence() + { + @Override + public int length() + { + return length; + } + + @Override + public char charAt(int index) + { + return string.charAt(offset + index); + } + + @Override + public CharSequence subSequence(int start, int end) + { + return WriteThroughWriter.subSequence(string, offset + start, end - start); + } + + @Override + public String toString() + { + return string.substring(offset, offset + length); + } + }; + } + + /** + * Get a zero copy subsequence of a {@code char} array. + * @param chars The characters to take a subsequence of. These character are not copied and the array should not be + * modified for the life of the returned CharSequence. + * @param offset The offset in characters into the string to start the subsequence + * @param length The length in characters of the substring + * @return A new {@link CharSequence} containing the subsequence. + */ + static CharSequence subSequence(char[] chars, int offset, int length) + { + // Needed to make bounds check of wrap the same as for string.substring + if (length == 0) + return ""; + return CharBuffer.wrap(chars, offset, length); + } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java index 0c4558f33205..8e45833b1921 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java @@ -59,23 +59,23 @@ public void testSimpleUTF8() throws Exception public void testUTF8() throws Exception { Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); - writer.write("How now Brown cow"); - assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); + writer.write("How now \uFF22rown cow"); + assertArrayEquals("How now \uFF22rown cow".getBytes(StandardCharsets.UTF_8), BufferUtil.toArray(_bytes)); } @Test public void testUTF16() throws Exception { Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_16); - writer.write("How now Brown cow"); - assertArrayEquals("How now Brown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); + writer.write("How now \uFF22rown cow"); + assertArrayEquals("How now \uFF22rown cow".getBytes(StandardCharsets.UTF_16), BufferUtil.toArray(_bytes)); } @Test public void testNotCESU8() throws Exception { Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); - String data = "xxx𐐀xxx"; + String data = "xxx\uD801\uDC00xxx"; writer.write(data); byte[] b = BufferUtil.toArray(_bytes); assertEquals("787878F0909080787878", StringUtil.toHexString(b)); @@ -93,7 +93,7 @@ public void testMultiByteOverflowUTF8() throws Exception Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); int maxWriteSize = WriteThroughWriter.DEFAULT_MAX_WRITE_SIZE; final String singleByteStr = "a"; - final String multiByteDuplicateStr = "B"; + final String multiByteDuplicateStr = "\uFF22"; int remainSize = 1; int multiByteStrByteLength = multiByteDuplicateStr.getBytes(StandardCharsets.UTF_8).length; @@ -115,7 +115,7 @@ public void testMultiByteOverflowUTF8() throws Exception public void testISO8859() throws Exception { Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.ISO_8859_1); - writer.write("How now Brown cow"); + writer.write("How now \uFF22rown cow"); assertEquals(new String(BufferUtil.toArray(_bytes), StandardCharsets.ISO_8859_1), "How now ?rown cow"); } @@ -123,7 +123,7 @@ public void testISO8859() throws Exception public void testUTF16x2() throws Exception { Writer writer = WriteThroughWriter.newWriter(_out, StandardCharsets.UTF_8); - String source = "𠮟"; + String source = "\uD842\uDF9F"; byte[] bytes = source.getBytes(StandardCharsets.UTF_8); writer.write(source.toCharArray(), 0, source.toCharArray().length); @@ -148,7 +148,7 @@ public void testMultiByteOverflowUTF16x2() throws Exception final String singleByteStr = "a"; int remainSize = 1; - final String multiByteDuplicateStr = "𠮟"; + final String multiByteDuplicateStr = "\uD842\uDF9F"; int adjustSize = -1; String source = @@ -179,7 +179,7 @@ public void testMultiByteOverflowUTF16X22() throws Exception final String singleByteStr = "a"; int remainSize = 1; - final String multiByteDuplicateStr = "𠮟"; + final String multiByteDuplicateStr = "\uD842\uDF9F"; int adjustSize = -2; String source = From 870ed722a3e7d891a10e796f3fe51fc87f858e1b Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 27 Sep 2023 07:20:14 +1000 Subject: [PATCH 14/15] Updates from review --- .../test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java index 8e45833b1921..f0d6ba185eb0 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/WriteThroughWriterTest.java @@ -35,6 +35,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck public class WriteThroughWriterTest { private OutputStream _out; From 27ebc12867e69ef8d05caf3b5157951bfa522871 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 27 Sep 2023 07:48:06 +1000 Subject: [PATCH 15/15] Updates from review --- .../handler/BufferedResponseHandlerTest.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java index acebc7ee9260..6c76433ab136 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BufferedResponseHandlerTest.java @@ -125,13 +125,16 @@ public void testExcludedByMime() throws Exception @Test public void testFlushed() throws Exception { + _test._writes = 4; _test._flush = true; _test._bufferSize = 2048; String response = _local.getResponse("GET /ctx/include/path HTTP/1.1\r\nHost: localhost\r\n\r\n"); assertThat(response, containsString(" 200 OK")); assertThat(response, containsString("Write: 0")); - assertThat(response, containsString("Write: 9")); - assertThat(response, containsString("Written: true")); + assertThat(response, containsString("Write: 1")); + assertThat(response, containsString("Transfer-Encoding: chunked")); + assertThat(response, not(containsString("Write: 3"))); + assertThat(response, not(containsString("Written: true"))); } @Test @@ -181,10 +184,10 @@ public void testFlushEmpty() throws Exception _test._content = new byte[0]; String response = _local.getResponse("GET /ctx/include/path HTTP/1.1\r\nHost: localhost\r\n\r\n"); assertThat(response, containsString(" 200 OK")); - assertThat(response, containsString("Content-Length: ")); + assertThat(response, containsString("Transfer-Encoding: chunked")); assertThat(response, containsString("Write: 0")); assertThat(response, not(containsString("Write: 1"))); - assertThat(response, containsString("Written: true")); + assertThat(response, not(containsString("Written: true"))); } @Test @@ -235,9 +238,11 @@ public boolean handle(Request request, Response response, Callback callback) thr { response.getHeaders().add("Write", Integer.toString(i)); outputStream.write(_content); - if (_flush) + if (_flush && i % 2 == 1) outputStream.flush(); } + if (_flush) + outputStream.flush(); response.getHeaders().add("Written", "true"); } callback.succeeded();