On Thu, Aug 22, 2024 at 2:33 PM <ma...@apache.org> wrote:
>
> 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 69eff83577 Expected behaviour has been clarified when writing >= c-l
> bytes to body
> 69eff83577 is described below
>
> commit 69eff83577f7c00cbaaca9384ab4b1989f516797
> 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
I'm not sure yet whether this is a good idea. Usually we want to keep
things open for further processing.
Maybe we should be doing it using the suspended flag instead and in the facades.
Rémy
> ---
> .../catalina/connector/LocalStrings.properties | 1 +
> .../apache/catalina/connector/OutputBuffer.java | 30 ++++++++++---
> .../catalina/connector/TestCoyoteOutputStream.java | 50
> ++++++++++++++++++++++
> 3 files changed, 76 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);
> + }
> + }
> + }
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
