This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push: new 2d931eed6c Add support for Servlet read/write via ByteBuffer 2d931eed6c is described below commit 2d931eed6c35702ef6541c0fd6c067b79e5eb371 Author: Mark Thomas <ma...@apache.org> AuthorDate: Mon Dec 12 15:43:28 2022 +0000 Add support for Servlet read/write via ByteBuffer --- java/jakarta/servlet/ServletInputStream.java | 141 ++++++++++++++++++++- java/jakarta/servlet/ServletOutputStream.java | 101 ++++++++++++++- .../catalina/connector/CoyoteInputStream.java | 14 +- .../catalina/connector/CoyoteOutputStream.java | 6 + .../org/apache/catalina/connector/InputBuffer.java | 4 + .../catalina/connector/LocalStrings.properties | 1 + java/org/apache/coyote/LocalStrings.properties | 1 - java/org/apache/coyote/Response.java | 5 +- .../apache/coyote/http11/Http11OutputBuffer.java | 7 + webapps/docs/changelog.xml | 11 ++ 10 files changed, 264 insertions(+), 27 deletions(-) diff --git a/java/jakarta/servlet/ServletInputStream.java b/java/jakarta/servlet/ServletInputStream.java index 0c8280c2e9..10048fbb88 100644 --- a/java/jakarta/servlet/ServletInputStream.java +++ b/java/jakarta/servlet/ServletInputStream.java @@ -18,6 +18,8 @@ package jakarta.servlet; import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; +import java.util.Objects; /** * Provides an input stream for reading binary data from a client request, @@ -43,6 +45,80 @@ public abstract class ServletInputStream extends InputStream { // NOOP } + /** + * Reads from the input stream into the given buffer. + * <p> + * If the input stream is in non-blocking mode, before each invocation of + * this method {@link #isReady()} must be called and must return + * {@code true} or the {@link ReadListener#onDataAvailable()} call back must + * indicate that data is available to read else an + * {@link IllegalStateException} must be thrown. + * <p> + * Otherwise, if this method is called when {@code buffer} has no space + * remaining, the method returns {@code 0} immediately and {@code buffer} is + * unchanged. + * <p> + * If the input stream is in blocking mode and {@code buffer} has space + * remaining, this method blocks until at least one byte has been read, end + * of stream is reached or an exception is thrown. + * <p> + * Returns the number of bytes read or {@code -1} if the end of stream is + * reached without reading any data. + * <p> + * When the method returns, and if data has been read, the buffer's position + * will be unchanged from the value when passed to this method and the limit + * will be the position incremented by the number of bytes read. + * <p> + * Subclasses are strongly encouraged to override this method and provide a + * more efficient implementation. + * + * @param buffer The buffer into which the data is read. + * + * @return The number of bytes read or {@code -1} if the end of the stream + * has been reached. + * + * @exception IllegalStateException If the input stream is in non-blocking + * mode and this method is called without first calling {@link #isReady()} + * and that method has returned {@code true} or + * {@link ReadListener#onDataAvailable()} has not signalled that data is + * available to read. + * + * @exception IOException If data cannot be read for any reason other than + * the end of stream being reached, the input stream has been closed or if + * some other I/O error occurs. + * + * @exception NullPointerException If buffer is null. + * + * @since Servlet 6.1 + */ + public int read(ByteBuffer buffer) throws IOException { + Objects.requireNonNull(buffer); + + if (!isReady()) { + throw new IllegalStateException(); + } + + if (buffer.remaining() == 0) { + return 0; + } + + byte[] b = new byte[buffer.remaining()]; + + int result = read(b); + if (result == -1) { + return -1; + } + + int position = buffer.position(); + + buffer.put(b, 0, result); + + buffer.position(position); + buffer.limit(position + result); + + return result; + } + /** * Reads the input stream, one line at a time. Starting at an offset, reads * bytes into an array, until it reads a certain number of bytes or reaches @@ -50,6 +126,8 @@ public abstract class ServletInputStream extends InputStream { * <p> * This method returns -1 if it reaches the end of the input stream before * reading the maximum number of bytes. + * <p> + * This method may only be used when the input stream is in blocking mode. * * @param b * an array of bytes into which data is read @@ -60,6 +138,10 @@ public abstract class ServletInputStream extends InputStream { * an integer specifying the maximum number of bytes to read * @return an integer specifying the actual number of bytes read, or -1 if * the end of the stream is reached + * + * @exception IllegalStateException + * If this method is called when the input stream is in + * non-blocking mode. * @exception IOException * if an input or output exception has occurred */ @@ -91,12 +173,25 @@ public abstract class ServletInputStream extends InputStream { public abstract boolean isFinished(); /** - * Can data be read from this InputStream without blocking? - * Returns If this method is called and returns false, the container will - * invoke {@link ReadListener#onDataAvailable()} when data is available. + * Returns {@code true} if it is allowable to call a {@code read()} method. + * In blocking mode, this method will always return {@code true}, but a + * subsequent call to a {@code read()} method may block awaiting data. In + * non-blocking mode this method may return {@code false}, in which case it + * is illegal to call a {@code read()} method and an + * {@link IllegalStateException} MUST be thrown. When + * {@link ReadListener#onDataAvailable()} is called, a call to this method + * that returned {@code true} is implicit. + * <p> + * If this method returns {@code false} and a {@link ReadListener} has been + * set via {@link #setReadListener(ReadListener)}, then the container will + * subsequently invoke {@link ReadListener#onDataAvailable()} (or + * {@link ReadListener#onAllDataRead()}) once data (or EOF) has become + * available. Other than the initial call + * {@link ReadListener#onDataAvailable()} will only be called if and only if + * this method is called and returns false. * - * @return <code>true</code> if data can be read without blocking, else - * <code>false</code> + * @return {@code true} if data can be obtained without blocking, otherwise + * returns {@code false}. * * @since Servlet 3.1 */ @@ -118,4 +213,40 @@ public abstract class ServletInputStream extends InputStream { * @since Servlet 3.1 */ public abstract void setReadListener(ReadListener listener); + + /** + * {@inheritDoc} + * <p> + * This method may only be used when the input stream is in blocking mode. + * + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode. + */ + @Override + public byte[] readAllBytes() throws IOException { + return super.readAllBytes(); + } + + /** + * {@inheritDoc} + * <p> + * This method may only be used when the input stream is in blocking mode. + * + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode. + */ + @Override + public byte[] readNBytes(int len) throws IOException { + return super.readNBytes(len); + } + + /** + * {@inheritDoc} + * <p> + * This method may only be used when the input stream is in blocking mode. + * + * @exception IllegalStateException If this method is called when the input stream is in non-blocking mode. + */ + @Override + public int readNBytes(byte[] b, int off, int len) throws IOException { + return super.readNBytes(b, off, len); + } } diff --git a/java/jakarta/servlet/ServletOutputStream.java b/java/jakarta/servlet/ServletOutputStream.java index 86a57c6602..66ee5c1503 100644 --- a/java/jakarta/servlet/ServletOutputStream.java +++ b/java/jakarta/servlet/ServletOutputStream.java @@ -19,7 +19,9 @@ package jakarta.servlet; import java.io.CharConversionException; import java.io.IOException; import java.io.OutputStream; +import java.nio.ByteBuffer; import java.text.MessageFormat; +import java.util.Objects; import java.util.ResourceBundle; /** @@ -45,6 +47,69 @@ public abstract class ServletOutputStream extends OutputStream { // NOOP } + /** + * Writes from the given buffer to the output stream. + * <p> + * If the output steam is in non-blocking mode, before each invocation of + * this method {@link #isReady()} must be called and must return + * {@code true} or the {@link WriteListener#onWritePossible()} call back + * must indicate that data may be written else an + * {@link IllegalStateException} must be thrown. + * <p> + * Otherwise, if this method is called when {@code buffer} has no data + * remaining, the method returns immediately and {@code buffer} is + * unchanged. + * <p> + * If the output stream is in non-blocking mode, neither the position, limit + * nor content of the buffer passed to this method may be modified until a + * subsequent call to {@link #isReady()} returns true or the + * {@link WriteListener#onWritePossible()} call back indicates data may be + * written again. At this point the buffer's limit will be unchanged from + * the value when passed to this method and the position will be the same as + * the limit. + * <p> + * If the output stream is in blocking mode and {@code buffer} has space + * remaining, this method blocks until all the remaining data in the buffer + * has been written. When the method returns, and if data has been written, + * the buffer's limit will be unchanged from the value when passed to this + * method and the position will be the same as the limit. + * <p> + * Subclasses are strongly encouraged to override this method and provide a + * more efficient implementation. + * + * @param buffer The buffer from which the data is written. + * + * @exception IllegalStateException If the output stream is in non-blocking + * mode and this method is called without first calling {@link #isReady()} + * and that method has returned {@code true} or + * {@link WriteListener#onWritePossible()} has not signalled that data may + * be written. + * + * @exception IOException If the output stream has been closed or if some + * other I/O error occurs. + * + * @exception NullPointerException If buffer is null. + * + * @since Servlet 6.1 + */ + public void write(ByteBuffer buffer) throws IOException { + Objects.requireNonNull(buffer); + + if (!isReady()) { + throw new IllegalStateException(); + } + + if (buffer.remaining() == 0) { + return; + } + + byte[] b = new byte[buffer.remaining()]; + + buffer.get(b); + + write(b); + } + /** * Writes a <code>String</code> to the client, without a carriage * return-line feed (CRLF) character at the end. @@ -278,13 +343,24 @@ public abstract class ServletOutputStream extends OutputStream { } /** - * Checks if a non-blocking write will succeed. If this returns - * <code>false</code>, it will cause a callback to - * {@link WriteListener#onWritePossible()} when the buffer has emptied. If - * this method returns <code>false</code> no further data must be written - * until the container calls {@link WriteListener#onWritePossible()}. + * Returns {@code true} if it is allowable to call any method that may write + * data (e.g. {@code write()}, {@code print()} or {@code flush}). In + * blocking mode, this method will always return {@code true}, but a + * subsequent call to a method that writes data may block. In non-blocking + * mode this method may return {@code false}, in which case it is illegal to + * call a method that writes data and an {@link IllegalStateException} MUST + * be thrown. When {@link WriteListener#onWritePossible()} is called, a call + * to this method that returned {@code true} is implicit. + * <p> + * If this method returns {@code false} and a {@link WriteListener} has been + * set via {@link #setWriteListener(WriteListener)}, then container will + * subsequently invoke {@link WriteListener#onWritePossible()} once a write + * operation becomes possible without blocking. Other than the initial call, + * {@link WriteListener#onWritePossible()} will only be called if and only + * if this method is called and returns false. * - * @return <code>true</code> if data can be written, else <code>false</code> + * @return {@code true} if data can be written without blocking, otherwise + * returns {@code false}. * * @since Servlet 3.1 */ @@ -306,4 +382,17 @@ public abstract class ServletOutputStream extends OutputStream { * @since Servlet 3.1 */ public abstract void setWriteListener(jakarta.servlet.WriteListener listener); + + /** + * {@inheritDoc} + * <p> + * If this method is called when the output stream is in non-blocking mode, it will immediately return with the stream + * effectively closed, even if the stream contains buffered data that is yet to be written to client. The container will + * write this data out in the background. If this process fails the {@link WriteListener#onError(Throwable)} method will + * be invoked as normal. + */ + @Override + public void close() throws IOException { + super.close(); + } } diff --git a/java/org/apache/catalina/connector/CoyoteInputStream.java b/java/org/apache/catalina/connector/CoyoteInputStream.java index ac3ce2d13c..01cc429903 100644 --- a/java/org/apache/catalina/connector/CoyoteInputStream.java +++ b/java/org/apache/catalina/connector/CoyoteInputStream.java @@ -21,6 +21,7 @@ import java.nio.ByteBuffer; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.util.Objects; import jakarta.servlet.ReadListener; import jakarta.servlet.ServletInputStream; @@ -134,18 +135,9 @@ public class CoyoteInputStream extends ServletInputStream { } - /** - * Transfers bytes from the buffer to the specified ByteBuffer. After the - * operation the position of the ByteBuffer will be returned to the one - * before the operation, the limit will be the position incremented by - * the number of the transferred bytes. - * - * @param b the ByteBuffer into which bytes are to be written. - * @return an integer specifying the actual number of bytes read, or -1 if - * the end of the stream is reached - * @throws IOException if an input or output exception has occurred - */ + @Override public int read(final ByteBuffer b) throws IOException { + Objects.requireNonNull(b); checkNonBlockingRead(); if (SecurityUtil.isPackageProtectionEnabled()) { diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java b/java/org/apache/catalina/connector/CoyoteOutputStream.java index ee005387b3..218fb00aaf 100644 --- a/java/org/apache/catalina/connector/CoyoteOutputStream.java +++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java @@ -18,6 +18,7 @@ package org.apache.catalina.connector; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.Objects; import jakarta.servlet.ServletOutputStream; import jakarta.servlet.WriteListener; @@ -100,8 +101,13 @@ public class CoyoteOutputStream extends ServletOutputStream { } + @Override public void write(ByteBuffer from) throws IOException { + Objects.requireNonNull(from); boolean nonBlocking = checkNonBlockingWrite(); + if (from.remaining() == 0) { + return; + } ob.write(from); if (nonBlocking) { checkRegisterForWrite(); diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java index ac1b816207..8d1f5f97c5 100644 --- a/java/org/apache/catalina/connector/InputBuffer.java +++ b/java/org/apache/catalina/connector/InputBuffer.java @@ -359,6 +359,10 @@ public class InputBuffer extends Reader public int read(ByteBuffer to) throws IOException { throwIfClosed(); + if (to.remaining() == 0) { + return 0; + } + if (checkByteBufferEof()) { return -1; } diff --git a/java/org/apache/catalina/connector/LocalStrings.properties b/java/org/apache/catalina/connector/LocalStrings.properties index 6726b4a303..09a1ed58f1 100644 --- a/java/org/apache/catalina/connector/LocalStrings.properties +++ b/java/org/apache/catalina/connector/LocalStrings.properties @@ -37,6 +37,7 @@ coyoteConnector.protocolHandlerStopFailed=Protocol handler stop failed coyoteInputStream.nbNotready=In non-blocking mode you may not read from the ServletInputStream until the previous read has completed and isReady() returns true coyoteInputStream.null=The input buffer object has been recycled and is no longer associated with this facade +coyoteInputStream.blockingOnly=This method may not be called when the input stream is in non-blocking mode (i.e. after a ReadListener has been configured) coyoteOutputStream.nbNotready=In non-blocking mode you may not write to the ServletOutputStream until the previous write has completed and isReady() returns true coyoteOutputStream.null=The output buffer object has been recycled and is no longer associated with this facade diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties index b708730f45..e06ffcd9a1 100644 --- a/java/org/apache/coyote/LocalStrings.properties +++ b/java/org/apache/coyote/LocalStrings.properties @@ -65,6 +65,5 @@ request.readListenerSet=The non-blocking read listener has already been set response.encoding.invalid=The encoding [{0}] is not recognised by the JRE response.noTrailers.notSupported=A trailer fields supplier may not be set for this response. Either the underlying protocol does not support trailer fields or the protocol requires that the supplier is set before the response is committed response.notAsync=It is only valid to switch to non-blocking IO within async processing or HTTP upgrade processing -response.notNonBlocking=It is invalid to call isReady() when the response has not been put into non-blocking mode response.nullWriteListener=The listener passed to setWriteListener() may not be null response.writeListenerSet=The non-blocking write listener has already been set diff --git a/java/org/apache/coyote/Response.java b/java/org/apache/coyote/Response.java index b3bd46022c..fadc3fc1c1 100644 --- a/java/org/apache/coyote/Response.java +++ b/java/org/apache/coyote/Response.java @@ -746,10 +746,7 @@ public final class Response { public boolean isReady() { if (listener == null) { - if (log.isDebugEnabled()) { - log.debug(sm.getString("response.notNonBlocking")); - } - return false; + return true; } // Assume write is not possible boolean ready = false; diff --git a/java/org/apache/coyote/http11/Http11OutputBuffer.java b/java/org/apache/coyote/http11/Http11OutputBuffer.java index cf666b605d..570b90c0d7 100644 --- a/java/org/apache/coyote/http11/Http11OutputBuffer.java +++ b/java/org/apache/coyote/http11/Http11OutputBuffer.java @@ -565,6 +565,13 @@ public class Http11OutputBuffer implements HttpOutputBuffer { @Override public void end() throws IOException { + /* + * TODO + * As of Servlet 6.1, this flush is (currently) meant to be + * non-blocking if the output stream is in non-blocking mode. That + * requirement creates various complications I want to discuss with + * the EG before I try implementing it. + */ socketWrapper.flush(true); } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 02755378c3..854f21011d 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -105,6 +105,17 @@ issues do not "pop up" wrt. others). --> <section name="Tomcat 11.0.0-M2 (markt)" rtext="in development"> + <subsection name="Catalina"> + <changelog> + <add> + Update the <code>ServletInputStream</code> and + <code>ServletOuputStream</code> classes in the Servlet API to align with + the recent updates in the Jakarta Servlet specification to support + reading and writing with <code>ByteBuffer</code>s. The changes also + clarified various aspects of the Servlet non-blocking API. (markt) + </add> + </changelog> + </subsection> <subsection name="Coyote"> <changelog> <fix> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org