This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/11.0.x by this push:
     new 168470d7a9 Expected behaviour has been clarified when writing >= c-l 
bytes to body
168470d7a9 is described below

commit 168470d7a97281c392aa71c3088ac3f6093e135a
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Aug 22 13:33:10 2024 +0100

    Expected behaviour has been clarified when writing >= c-l bytes to body
---
 .../catalina/connector/LocalStrings.properties     |  1 +
 .../apache/catalina/connector/OutputBuffer.java    | 30 ++++++++++---
 .../catalina/connector/TestCoyoteOutputStream.java | 50 ++++++++++++++++++++++
 webapps/docs/changelog.xml                         | 10 +++++
 4 files changed, 86 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/catalina/connector/LocalStrings.properties 
b/java/org/apache/catalina/connector/LocalStrings.properties
index 077cf46136..d20cd26bc4 100644
--- a/java/org/apache/catalina/connector/LocalStrings.properties
+++ b/java/org/apache/catalina/connector/LocalStrings.properties
@@ -83,6 +83,7 @@ coyoteResponse.setBufferSize.ise=Cannot change buffer size 
after data has been w
 inputBuffer.requiresNonBlocking=Not available in non blocking mode
 inputBuffer.streamClosed=Stream closed
 
+outputBuffer.closed=The response may not be written to once it has been closed
 outputBuffer.writeNull=The String argument to write(String,int,int) may not be 
null
 
 request.asyncNotSupported=A filter or servlet of the current chain does not 
support asynchronous operations.
diff --git a/java/org/apache/catalina/connector/OutputBuffer.java 
b/java/org/apache/catalina/connector/OutputBuffer.java
index b185561ef3..04cf82e632 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -358,11 +358,11 @@ public class OutputBuffer extends Writer {
     private void writeBytes(byte b[], int off, int len) throws IOException {
 
         if (closed) {
-            return;
+            throw new IOException(sm.getString("outputBuffer.closed"));
         }
 
         append(b, off, len);
-        bytesWritten += len;
+        updateBytesWritten(len);
 
         // if called from within flush(), then immediately flush
         // remaining bytes
@@ -376,12 +376,12 @@ public class OutputBuffer extends Writer {
     private void writeBytes(ByteBuffer from) throws IOException {
 
         if (closed) {
-            return;
+            throw new IOException(sm.getString("outputBuffer.closed"));
         }
 
         int remaining = from.remaining();
         append(from);
-        bytesWritten += remaining;
+        updateBytesWritten(remaining);
 
         // if called from within flush(), then immediately flush
         // remaining bytes
@@ -394,6 +394,10 @@ public class OutputBuffer extends Writer {
 
     public void writeByte(int b) throws IOException {
 
+        if (closed) {
+            throw new IOException(sm.getString("outputBuffer.closed"));
+        }
+
         if (suspended) {
             return;
         }
@@ -403,8 +407,24 @@ public class OutputBuffer extends Writer {
         }
 
         transfer((byte) b, bb);
-        bytesWritten++;
+        updateBytesWritten(1);
+    }
+
 
+    private void updateBytesWritten(int increment) throws IOException {
+        bytesWritten += increment;
+        int contentLength = coyoteResponse.getContentLength();
+
+        /*
+         * Handle the requirements of section 5.7 of the Servlet specification 
- Closure of the Response Object.
+         *
+         * Currently this just handles the simple case. There is work in 
progress to better define what should happen if
+         * an attempt is made to write > content-length bytes. When that work 
is complete, this is likely where the
+         * implementation will end up.
+         */
+        if (contentLength != -1 && bytesWritten >= contentLength) {
+            close();
+        }
     }
 
 
diff --git a/test/org/apache/catalina/connector/TestCoyoteOutputStream.java 
b/test/org/apache/catalina/connector/TestCoyoteOutputStream.java
index d808db76f7..bf282f5d67 100644
--- a/test/org/apache/catalina/connector/TestCoyoteOutputStream.java
+++ b/test/org/apache/catalina/connector/TestCoyoteOutputStream.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.RandomAccessFile;
 import java.nio.channels.FileChannel.MapMode;
 import java.nio.charset.StandardCharsets;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import jakarta.servlet.AsyncContext;
@@ -290,4 +291,53 @@ public class TestCoyoteOutputStream extends TomcatBaseTest 
{
         }
 
     }
+
+
+    @Test
+    public void testWriteAfterBodyComplete() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        Context root = tomcat.addContext("", TEMP_DIR);
+        Tomcat.addServlet(root, "servlet", new 
WriteAfterBodyCompleteServlet());
+        root.addServletMappingDecoded("/", "servlet");
+
+        tomcat.start();
+
+        ByteChunk bc = new ByteChunk();
+        int rc = getUrl("http://localhost:"; + getPort() + "/", bc, null, null);
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+
+        // Wait upto 5s
+        int count = 0;
+        boolean exceptionSeen = 
WriteAfterBodyCompleteServlet.exceptionSeen.get();
+        while (count < 50 && !exceptionSeen) {
+            Thread.sleep(100);
+            exceptionSeen = WriteAfterBodyCompleteServlet.exceptionSeen.get();
+            count++;
+        }
+        Assert.assertTrue(exceptionSeen);
+    }
+
+
+    private static final class WriteAfterBodyCompleteServlet extends 
HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+        private static final AtomicBoolean exceptionSeen = new 
AtomicBoolean(false);
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) 
throws ServletException, IOException {
+            resp.setCharacterEncoding(StandardCharsets.UTF_8);
+            resp.setContentType("text/plain");
+            resp.setContentLengthLong(10);
+            ServletOutputStream sos = resp.getOutputStream();
+            sos.write("0123456789".getBytes(StandardCharsets.UTF_8));
+
+            // sos should now be closed. A further write should trigger an 
error.
+            try {
+                sos.write("anything".getBytes(StandardCharsets.UTF_8));
+            } catch (IOException ioe) {
+                exceptionSeen.set(true);
+            }
+        }
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1e642a34e0..c1d36e3fe3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,16 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 11.0.0-M25 (markt)" rtext="in development">
+  <subsection name="Catalina">
+    <changelog>
+      <add>
+        Implement the recent clarification from the Jakarta Servlet project 
that
+        if a content length is declared then once that many bytes have been
+        written to the response, further writes should trigger an
+        <code>IOException</code>. (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