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

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


The following commit(s) were added to refs/heads/8.5.x by this push:
     new fda5196f31 Align error handling for CoyoteWriter and CoyoteOutputStream
fda5196f31 is described below

commit fda5196f317d3d014b35969029ccddffd022563c
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 29 09:10:19 2024 +0000

    Align error handling for CoyoteWriter and CoyoteOutputStream
    
    Prior to this change it was possible for errors on the
    CoyoteOutputStream to be recorded against the wrong response if the
    response was recycled during the call to realWriteBytes().
---
 .../catalina/connector/CoyoteOutputStream.java     | 35 ++++++++++++++++++----
 .../apache/catalina/connector/CoyoteWriter.java    | 12 +++++---
 .../apache/catalina/connector/OutputBuffer.java    |  7 ++++-
 webapps/docs/changelog.xml                         | 11 +++++++
 4 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/java/org/apache/catalina/connector/CoyoteOutputStream.java 
b/java/org/apache/catalina/connector/CoyoteOutputStream.java
index a6cb729b9e..40209b352e 100644
--- a/java/org/apache/catalina/connector/CoyoteOutputStream.java
+++ b/java/org/apache/catalina/connector/CoyoteOutputStream.java
@@ -78,7 +78,12 @@ public class CoyoteOutputStream extends ServletOutputStream {
     @Override
     public void write(int i) throws IOException {
         boolean nonBlocking = checkNonBlockingWrite();
-        ob.writeByte(i);
+        try {
+            ob.writeByte(i);
+        } catch (IOException ioe) {
+            ob.setErrorException(ioe);
+            throw ioe;
+        }
         if (nonBlocking) {
             checkRegisterForWrite();
         }
@@ -94,7 +99,12 @@ public class CoyoteOutputStream extends ServletOutputStream {
     @Override
     public void write(byte[] b, int off, int len) throws IOException {
         boolean nonBlocking = checkNonBlockingWrite();
-        ob.write(b, off, len);
+        try {
+            ob.write(b, off, len);
+        } catch (IOException ioe) {
+            ob.setErrorException(ioe);
+            throw ioe;
+        }
         if (nonBlocking) {
             checkRegisterForWrite();
         }
@@ -104,7 +114,12 @@ public class CoyoteOutputStream extends 
ServletOutputStream {
     public void write(ByteBuffer from) throws IOException {
         Objects.requireNonNull(from);
         boolean nonBlocking = checkNonBlockingWrite();
-        ob.write(from);
+        try {
+            ob.write(from);
+        } catch (IOException ioe) {
+            ob.setErrorException(ioe);
+            throw ioe;
+        }
         if (nonBlocking) {
             checkRegisterForWrite();
         }
@@ -117,7 +132,12 @@ public class CoyoteOutputStream extends 
ServletOutputStream {
     @Override
     public void flush() throws IOException {
         boolean nonBlocking = checkNonBlockingWrite();
-        ob.flush();
+        try {
+            ob.flush();
+        } catch (IOException ioe) {
+            ob.setErrorException(ioe);
+            throw ioe;
+        }
         if (nonBlocking) {
             checkRegisterForWrite();
         }
@@ -152,7 +172,12 @@ public class CoyoteOutputStream extends 
ServletOutputStream {
 
     @Override
     public void close() throws IOException {
-        ob.close();
+        try {
+            ob.close();
+        } catch (IOException ioe) {
+            ob.setErrorException(ioe);
+            throw ioe;
+        }
     }
 
     @Override
diff --git a/java/org/apache/catalina/connector/CoyoteWriter.java 
b/java/org/apache/catalina/connector/CoyoteWriter.java
index 6eef35949d..6868156f0a 100644
--- a/java/org/apache/catalina/connector/CoyoteWriter.java
+++ b/java/org/apache/catalina/connector/CoyoteWriter.java
@@ -92,7 +92,7 @@ public class CoyoteWriter extends PrintWriter {
         try {
             ob.flush();
         } catch (IOException e) {
-            error = true;
+            setErrorException(e);
         }
 
     }
@@ -130,7 +130,7 @@ public class CoyoteWriter extends PrintWriter {
         try {
             ob.write(c);
         } catch (IOException e) {
-            error = true;
+            setErrorException(e);
         }
 
     }
@@ -146,7 +146,7 @@ public class CoyoteWriter extends PrintWriter {
         try {
             ob.write(buf, off, len);
         } catch (IOException e) {
-            error = true;
+            setErrorException(e);
         }
 
     }
@@ -168,7 +168,7 @@ public class CoyoteWriter extends PrintWriter {
         try {
             ob.write(s, off, len);
         } catch (IOException e) {
-            error = true;
+            setErrorException(e);
         }
 
     }
@@ -313,4 +313,8 @@ public class CoyoteWriter extends PrintWriter {
     }
 
 
+    private void setErrorException(Exception e) {
+        error = true;
+        ob.setErrorException(e);
+    }
 }
diff --git a/java/org/apache/catalina/connector/OutputBuffer.java 
b/java/org/apache/catalina/connector/OutputBuffer.java
index e1aac60285..e945516f02 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -362,7 +362,6 @@ public class OutputBuffer extends Writer {
                 // An IOException on a write is almost always due to
                 // the remote client aborting the request. Wrap this
                 // so that it can be handled better by the error dispatcher.
-                coyoteResponse.setErrorException(e);
                 throw new ClientAbortException(e);
             }
         }
@@ -799,6 +798,12 @@ public class OutputBuffer extends Writer {
         }
     }
 
+
+    public void setErrorException(Exception e) {
+        coyoteResponse.setErrorException(e);
+    }
+
+
     private void appendByteArray(byte src[], int off, int len) throws 
IOException {
         if (len == 0) {
             return;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 78cd4aa7ce..4313ee557f 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 8.5.100 (schultz)" rtext="in development">
+  <subsection name="Catalina">
+    <changelog>
+      <fix>
+        Align error handling for <code>Writer</code> and
+        <code>OutputStream</code>. Ensure use of either once the response has
+        been recycled triggers a <code>NullPointerException</code> provided 
that
+        <code>discardFacades</code> is configured with the default value of
+        <code>true</code>.
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Jasper">
     <changelog>
       <add>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to