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

Reply via email to