You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/09/30 09:35:45 UTC

(tomcat) branch 11.0.x updated (9b0f7e4cfe -> 731e38b9c8)

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

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


    from 9b0f7e4cfe Follow-up to 0dd86e53
     new dd0febe2f9 Fix InputBuffer.skip() consuming and returning more characters than requested by comparing and advancing by the remaining count (n - nRead) rather than the original count, and add a regression test
     new 528a5b0883 Record an error in CoyoteWriter.close() when the underlying OutputBuffer close fails, rather than swallowing the IOException silently, so checkError() reflects the failure
     new 0c9837fd06 Return 0 rather than -1 for zero-length reads from InputBuffer at end of stream, as required by the InputStream/Reader contract, and avoid the unnecessary blocking fill these requests previously triggered
     new 7ae6a1150a Return a snapshot enumeration from Request.getLocales() to prevent ConcurrentModificationExceptions when the locales list is modified while a previously returned enumeration is being iterated
     new 5027181e52 Add spec-mandated validation to ApplicationServletRegistration: throw IllegalStateException when the registration is modified after the context has been initialised (using a deliberately lenient availability test to avoid breaking existing applications that configure registrations before start) and IllegalArgumentException from addMapping for null or empty URL patterns
     new 1b2ad67e5f Fall back to the raw trimmed filename value in ApplicationPart.getSubmittedFileName() when HttpParser.unquote() rejects an invalid quoted-string, keeping the result consistent with the file-vs-field classification performed by FileUploadBase.getFileName() for malformed Content-Disposition headers
     new 67ceed2eae Make FrameworkListener registration idempotent so repeated Server start events do not add duplicate container listeners or orphan previous per-context lifecycle listeners that can no longer be removed
     new 731e38b9c8 Merge branch '11.0.x' of git@github.com:apache/tomcat.git into 11.0.x

The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/catalina/connector/CoyoteWriter.java    |   8 +-
 .../org/apache/catalina/connector/InputBuffer.java |  12 ++-
 java/org/apache/catalina/connector/Request.java    |   4 +-
 java/org/apache/catalina/core/ApplicationPart.java |   7 +-
 .../core/ApplicationServletRegistration.java       |  25 ++++-
 .../apache/catalina/core/FrameworkListener.java    |  28 +++++-
 .../apache/catalina/core/LocalStrings.properties   |   2 +
 .../apache/catalina/connector/TestInputBuffer.java | 100 +++++++++++++++++++
 .../apache/catalina/core/TestApplicationPart.java  |  44 +++++++++
 .../core/TestApplicationServletRegistration.java   | 107 ++++++++++++++++++++-
 .../catalina/core/TestFrameworkListener.java       |  99 +++++++++++++++++++
 11 files changed, 424 insertions(+), 12 deletions(-)
 create mode 100644 test/org/apache/catalina/core/TestApplicationPart.java
 create mode 100644 test/org/apache/catalina/core/TestFrameworkListener.java


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


(tomcat) 05/08: Add spec-mandated validation to ApplicationServletRegistration: throw IllegalStateException when the registration is modified after the context has been initialised (using a deliberately lenient availability test to avoid breaking existing applications that configure registrations before start) and IllegalArgumentException from addMapping for null or empty URL patterns

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 5027181e5213d059de2299a50b27438b59aa7d52
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 11:08:50 2026 +0200

    Add spec-mandated validation to ApplicationServletRegistration: throw
    IllegalStateException when the registration is modified after the
    context has been initialised (using a deliberately lenient availability
    test to avoid breaking existing applications that configure
    registrations before start) and IllegalArgumentException from addMapping
    for null or empty URL patterns
---
 .../core/ApplicationServletRegistration.java       |  25 ++++-
 .../apache/catalina/core/LocalStrings.properties   |   2 +
 .../core/TestApplicationServletRegistration.java   | 107 ++++++++++++++++++++-
 3 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/core/ApplicationServletRegistration.java b/java/org/apache/catalina/core/ApplicationServletRegistration.java
index 981ce6f50e..f5bd7ebe72 100644
--- a/java/org/apache/catalina/core/ApplicationServletRegistration.java
+++ b/java/org/apache/catalina/core/ApplicationServletRegistration.java
@@ -95,6 +95,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
             throw new IllegalArgumentException(
                     sm.getString("applicationServletRegistration.nullInitParam", name, value));
         }
+        checkState();
         if (getInitParameter(name) != null) {
             return false;
         }
@@ -119,6 +120,8 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
             }
         }
 
+        checkState();
+
         // Have to add in a separate loop since spec requires no updates at all
         // if there is an issue
         if (conflicts.isEmpty()) {
@@ -132,6 +135,7 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
 
     @Override
     public void setAsyncSupported(boolean asyncSupported) {
+        checkState();
         wrapper.setAsyncSupported(asyncSupported);
     }
 
@@ -170,8 +174,9 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
     @SuppressWarnings("deprecation")
     @Override
     public Set<String> addMapping(String... urlPatterns) {
-        if (urlPatterns == null) {
-            return Collections.emptySet();
+        if (urlPatterns == null || urlPatterns.length == 0) {
+            throw new IllegalArgumentException(
+                    sm.getString("applicationServletRegistration.nullUrlPatterns", getName(), context.getName()));
         }
 
         String[] decodedUrlPatterns = new String[urlPatterns.length];
@@ -186,6 +191,8 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
             }
         }
 
+        checkState();
+
         Set<String> conflicts = new HashSet<>();
         Set<String> overrides = new HashSet<>();
 
@@ -246,4 +253,18 @@ public class ApplicationServletRegistration implements ServletRegistration.Dynam
         return wrapper.getRunAs();
     }
 
+
+    private void checkState() {
+        // The specification requires an IllegalStateException if the
+        // ServletContext has already been initialised. A stricter check
+        // (only allowing calls during STARTING_PREP, as used by
+        // setServletSecurity()) risks breaking existing applications that
+        // configure registrations before the context starts, so the looser
+        // test below only rejects calls made once the context is available.
+        if (context.getState().isAvailable()) {
+            throw new IllegalStateException(sm.getString("applicationServletRegistration.ise", getName(),
+                    context.getName()));
+        }
+    }
+
 }
diff --git a/java/org/apache/catalina/core/LocalStrings.properties b/java/org/apache/catalina/core/LocalStrings.properties
index 4b09e016b5..803c1a0716 100644
--- a/java/org/apache/catalina/core/LocalStrings.properties
+++ b/java/org/apache/catalina/core/LocalStrings.properties
@@ -76,6 +76,8 @@ applicationServletRegistration.nullInitParams=Unable to set initialisation param
 applicationServletRegistration.nullUrlPattern=URL patterns must not be null or empty
 applicationServletRegistration.setServletSecurity.iae=Null constraint specified for servlet [{0}] deployed to context with name [{1}]
 applicationServletRegistration.setServletSecurity.ise=Security constraints can''t be added to servlet [{0}] deployed to context with name [{1}] as the context has already been initialised
+applicationServletRegistration.ise=Servlet [{0}] deployed to context with name [{1}] can''t be modified after the context has been initialised
+applicationServletRegistration.nullUrlPatterns=Null or empty URL patterns specified for servlet [{0}] deployed to context with name [{1}]
 
 applicationSessionCookieConfig.ise=Property [{0}] cannot be added to SessionCookieConfig for context [{1}] as the context has been initialised
 
diff --git a/test/org/apache/catalina/core/TestApplicationServletRegistration.java b/test/org/apache/catalina/core/TestApplicationServletRegistration.java
index 4452a0baf9..8cb90c3257 100644
--- a/test/org/apache/catalina/core/TestApplicationServletRegistration.java
+++ b/test/org/apache/catalina/core/TestApplicationServletRegistration.java
@@ -16,12 +16,19 @@
  */
 package org.apache.catalina.core;
 
+import jakarta.servlet.ServletRegistration;
+
 import org.junit.Assert;
 import org.junit.Test;
 
+import org.apache.catalina.Context;
+import org.apache.catalina.LifecycleState;
 import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.TesterServlet;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
 
-public class TestApplicationServletRegistration {
+public class TestApplicationServletRegistration extends TomcatBaseTest {
 
     @Test
     public void testUrlPatternEncoded() {
@@ -45,4 +52,102 @@ public class TestApplicationServletRegistration {
         Assert.assertTrue(registration.addMapping("/servlet%25").isEmpty());
         Assert.assertEquals("servlet", context.findServletMapping(expectedPattern));
     }
+
+
+    @Test
+    public void testAddMappingNullAndEmptyPatterns() {
+        StandardContext context = new StandardContext();
+
+        Wrapper wrapper = context.createWrapper();
+        wrapper.setName("servlet");
+        context.addChild(wrapper);
+
+        ApplicationServletRegistration registration = new ApplicationServletRegistration(wrapper, context);
+
+        try {
+            registration.addMapping((String[]) null);
+            Assert.fail("Expected an IllegalArgumentException for null patterns");
+        } catch (IllegalArgumentException e) {
+            // Expected
+        }
+
+        try {
+            registration.addMapping();
+            Assert.fail("Expected an IllegalArgumentException for an empty pattern array");
+        } catch (IllegalArgumentException e) {
+            // Expected
+        }
+
+        try {
+            registration.addMapping("");
+            Assert.fail("Expected an IllegalArgumentException for an empty pattern");
+        } catch (IllegalArgumentException e) {
+            // Expected
+        }
+    }
+
+
+    @Test
+    public void testAddMappingWhileContextAvailable() {
+        CustomContext context = new CustomContext();
+        context.setState(LifecycleState.NEW);
+
+        Wrapper wrapper = context.createWrapper();
+        wrapper.setName("servlet");
+        context.addChild(wrapper);
+
+        context.setState(LifecycleState.STARTED);
+
+        ApplicationServletRegistration registration = new ApplicationServletRegistration(wrapper, context);
+
+        try {
+            registration.addMapping("/servlet");
+            Assert.fail("Expected an IllegalStateException once the context is available");
+        } catch (IllegalStateException e) {
+            // Expected
+        }
+    }
+
+
+    @Test
+    public void testModificationAfterContextInitialised() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        Context root = getProgrammaticRootContext();
+        Tomcat.addServlet(root, "servlet", new TesterServlet());
+        root.addServletMapping("/test", "servlet");
+
+        tomcat.start();
+
+        ServletRegistration registration =
+                root.getServletContext().getServletRegistration("servlet");
+
+        try {
+            registration.setInitParameter("param", "value");
+            Assert.fail("Expected an IllegalStateException after initialisation");
+        } catch (IllegalStateException e) {
+            // Expected
+        }
+
+        try {
+            ((ServletRegistration.Dynamic) registration).addMapping("/other");
+            Assert.fail("Expected an IllegalStateException after initialisation");
+        } catch (IllegalStateException e) {
+            // Expected
+        }
+    }
+
+
+    private static class CustomContext extends StandardContext {
+        private volatile LifecycleState state;
+
+        @Override
+        public LifecycleState getState() {
+            return state;
+        }
+
+        @Override
+        public synchronized void setState(LifecycleState state) {
+            this.state = state;
+        }
+    }
 }


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


(tomcat) 07/08: Make FrameworkListener registration idempotent so repeated Server start events do not add duplicate container listeners or orphan previous per-context lifecycle listeners that can no longer be removed

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 67ceed2eae1038cc92d18a7784ea1ffe3edf5936
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 11:18:51 2026 +0200

    Make FrameworkListener registration idempotent so repeated Server start events do not add duplicate container listeners or orphan previous per-context lifecycle listeners that can no longer be removed
---
 .../apache/catalina/core/FrameworkListener.java    | 28 +++++-
 .../catalina/core/TestFrameworkListener.java       | 99 ++++++++++++++++++++++
 2 files changed, 124 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/core/FrameworkListener.java b/java/org/apache/catalina/core/FrameworkListener.java
index c8b8c95cf3..e755cb325b 100644
--- a/java/org/apache/catalina/core/FrameworkListener.java
+++ b/java/org/apache/catalina/core/FrameworkListener.java
@@ -83,12 +83,29 @@ public abstract class FrameworkListener implements LifecycleListener, ContainerL
         for (Service service : server.findServices()) {
             Engine engine = service.getContainer();
             if (engine != null) {
-                engine.addContainerListener(this);
+                addContainerListenerIfAbsent(engine);
                 registerListenersForEngine(engine);
             }
         }
     }
 
+
+    /**
+     * Registers this as a container listener on the given container unless it is already registered, so that repeated
+     * Server start events do not result in duplicate registrations.
+     *
+     * @param container The container on which to register this listener
+     */
+    private void addContainerListenerIfAbsent(Container container) {
+        for (ContainerListener listener : container.findContainerListeners()) {
+            if (listener == this) {
+                return;
+            }
+        }
+        container.addContainerListener(this);
+    }
+
+
     /**
      * Registers listeners on all hosts of the given engine.
      *
@@ -97,11 +114,10 @@ public abstract class FrameworkListener implements LifecycleListener, ContainerL
     protected void registerListenersForEngine(Engine engine) {
         for (Container hostContainer : engine.findChildren()) {
             Host host = (Host) hostContainer;
-            host.addContainerListener(this);
+            addContainerListenerIfAbsent(host);
             registerListenersForHost(host);
         }
     }
-
     /**
      * Registers listeners on all contexts of the given host.
      *
@@ -120,6 +136,12 @@ public abstract class FrameworkListener implements LifecycleListener, ContainerL
      * @param context The context to register a listener for
      */
     protected void registerContextListener(Context context) {
+        // Reuse an existing listener if one is already registered, so that
+        // repeated Server start events do not create duplicate listeners that
+        // stay attached to the context after the map reference is replaced
+        if (contextListeners.containsKey(context)) {
+            return;
+        }
         LifecycleListener listener = createLifecycleListener(context);
         contextListeners.put(context, listener);
         context.addLifecycleListener(listener);
diff --git a/test/org/apache/catalina/core/TestFrameworkListener.java b/test/org/apache/catalina/core/TestFrameworkListener.java
new file mode 100644
index 0000000000..d8b59df1c6
--- /dev/null
+++ b/test/org/apache/catalina/core/TestFrameworkListener.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.core;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Stream;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.ContainerListener;
+import org.apache.catalina.Context;
+import org.apache.catalina.Lifecycle;
+import org.apache.catalina.LifecycleEvent;
+import org.apache.catalina.LifecycleListener;
+
+public class TestFrameworkListener {
+
+    @Test
+    public void testRegistrationIsIdempotent() throws Exception {
+        StandardServer server = new StandardServer();
+        StandardService service = new StandardService();
+        service.setName("service");
+        StandardEngine engine = new StandardEngine();
+        engine.setName("engine");
+        service.setContainer(engine);
+        server.addService(service);
+
+        StandardHost host = new StandardHost();
+        host.setName("localhost");
+        engine.addChild(host);
+
+        StandardContext context = new StandardContext();
+        context.setPath("/ctx");
+        host.addChild(context);
+
+        TesterListener listener = new TesterListener();
+
+        LifecycleEvent beforeStart = new LifecycleEvent(server, Lifecycle.BEFORE_START_EVENT, null);
+
+        // Simulate repeated in-place Server starts
+        listener.lifecycleEvent(beforeStart);
+        listener.lifecycleEvent(beforeStart);
+        listener.lifecycleEvent(beforeStart);
+
+        Assert.assertEquals(1, countListener(engine.findContainerListeners(), listener));
+        Assert.assertEquals(1, countListener(host.findContainerListeners(), listener));
+        Assert.assertEquals(1, listener.getContextListenerCount());
+        Assert.assertEquals(1, listener.getCreatedListeners().size());
+        // Exactly one listener — the one created for this context — must be attached
+        LifecycleListener created = listener.getCreatedListeners().get(0);
+        long attached = Stream.of(context.findLifecycleListeners()).filter(l -> l == created).count();
+        Assert.assertEquals(1, attached);
+    }
+
+
+    private static long countListener(ContainerListener[] listeners, ContainerListener expected) {
+        return Stream.of(listeners).filter(l -> l == expected).count();
+    }
+
+
+    private static class TesterListener extends FrameworkListener {
+
+        private final List<LifecycleListener> createdListeners = new ArrayList<>();
+
+        @Override
+        protected LifecycleListener createLifecycleListener(Context context) {
+            // Return a new instance on each call, as OpenWebBeansListener does
+            LifecycleListener listener = event -> {
+                // No-Op
+            };
+            createdListeners.add(listener);
+            return listener;
+        }
+
+        int getContextListenerCount() {
+            return contextListeners.size();
+        }
+
+        List<LifecycleListener> getCreatedListeners() {
+            return createdListeners;
+        }
+    }
+}


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


(tomcat) 06/08: Fall back to the raw trimmed filename value in ApplicationPart.getSubmittedFileName() when HttpParser.unquote() rejects an invalid quoted-string, keeping the result consistent with the file-vs-field classification performed by FileUploadBase.getFileName() for malformed Content-Disposition headers

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1b2ad67e5ff4cf9ea181ef7cd749dd7681554312
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 11:12:02 2026 +0200

    Fall back to the raw trimmed filename value in ApplicationPart.getSubmittedFileName() when HttpParser.unquote() rejects an invalid quoted-string, keeping the result consistent with the file-vs-field classification performed by FileUploadBase.getFileName() for malformed Content-Disposition headers
---
 java/org/apache/catalina/core/ApplicationPart.java |  7 +++-
 .../apache/catalina/core/TestApplicationPart.java  | 44 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/java/org/apache/catalina/core/ApplicationPart.java b/java/org/apache/catalina/core/ApplicationPart.java
index 95a7167b19..db523e22cd 100644
--- a/java/org/apache/catalina/core/ApplicationPart.java
+++ b/java/org/apache/catalina/core/ApplicationPart.java
@@ -161,7 +161,12 @@ public class ApplicationPart implements Part {
                         // RFC 6266. This is either a token or a quoted-string
                         if (fileName.indexOf('\\') > -1) {
                             // This is a quoted-string
-                            fileName = HttpParser.unquote(fileName.trim());
+                            String unquoted = HttpParser.unquote(fileName.trim());
+                            // If the quoted-string was invalid (e.g. ended
+                            // with a backslash) fall back to the raw value to
+                            // remain consistent with the classifier used by
+                            // the multipart parser (FileUploadBase.getFileName)
+                            fileName = unquoted != null ? unquoted : fileName.trim();
                         } else {
                             // This is a token
                             fileName = fileName.trim();
diff --git a/test/org/apache/catalina/core/TestApplicationPart.java b/test/org/apache/catalina/core/TestApplicationPart.java
new file mode 100644
index 0000000000..6b278a1216
--- /dev/null
+++ b/test/org/apache/catalina/core/TestApplicationPart.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.core;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.http.fileupload.disk.DiskFileItem;
+import org.apache.tomcat.util.http.fileupload.util.FileItemHeadersImpl;
+
+public class TestApplicationPart {
+
+    private static ApplicationPart createPart(String contentDisposition) {
+        DiskFileItem fileItem = new DiskFileItem("field", "text/plain", false, "name", 102400, null);
+        FileItemHeadersImpl headers = new FileItemHeadersImpl();
+        headers.addHeader("Content-Disposition", contentDisposition);
+        fileItem.setHeaders(headers);
+        return new ApplicationPart(fileItem, null);
+    }
+
+
+    @Test
+    public void testSubmittedFileNameUnterminatedQuotedString() {
+        // An escaped closing quote makes the quoted-string invalid.
+        // getSubmittedFileName() must not diverge (return null) from the
+        // classifier used by the multipart parser (which yields "abc\").
+        ApplicationPart part = createPart("form-data; name=\"x\"; filename=\"abc\\\"");
+        Assert.assertEquals("abc\\", part.getSubmittedFileName());
+    }
+}


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


(tomcat) 01/08: Fix InputBuffer.skip() consuming and returning more characters than requested by comparing and advancing by the remaining count (n - nRead) rather than the original count, and add a regression test

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit dd0febe2f98b2cf53813d8152780eb8f5c93ac00
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 10:44:48 2026 +0200

    Fix InputBuffer.skip() consuming and returning more characters than requested by comparing and advancing by the remaining count (n - nRead) rather than the original count, and add a regression test
---
 .../org/apache/catalina/connector/InputBuffer.java |  4 +-
 .../apache/catalina/connector/TestInputBuffer.java | 61 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java
index 10fe74c300..5fc9285b5e 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -520,8 +520,8 @@ public class InputBuffer extends Reader implements ByteChunk.ByteInputChannel, A
 
         long nRead = 0;
         while (nRead < n) {
-            if (cb.remaining() >= n) {
-                cb.position(cb.position() + (int) n);
+            if (cb.remaining() >= (n - nRead)) {
+                cb.position(cb.position() + (int) (n - nRead));
                 nRead = n;
             } else {
                 nRead += cb.remaining();
diff --git a/test/org/apache/catalina/connector/TestInputBuffer.java b/test/org/apache/catalina/connector/TestInputBuffer.java
index 55d3ad142f..bb7ce5ce76 100644
--- a/test/org/apache/catalina/connector/TestInputBuffer.java
+++ b/test/org/apache/catalina/connector/TestInputBuffer.java
@@ -23,6 +23,9 @@ import java.io.Writer;
 import java.nio.charset.MalformedInputException;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServlet;
@@ -102,6 +105,36 @@ public class TestInputBuffer extends TomcatBaseTest {
     }
 
 
+    @Test
+    public void testSkip() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        Context root = tomcat.addContext("", TEMP_DIR);
+        Tomcat.addServlet(root, "Skip", new SkipServlet());
+        root.addServletMapping("/test", "Skip");
+
+        // Limit socket reads to 10 bytes so skip() has to refill repeatedly
+        Assert.assertTrue(tomcat.getConnector().setProperty("socket.appReadBufSize", "10"));
+
+        tomcat.start();
+
+        StringBuilder body = new StringBuilder();
+        for (int i = 0; i < 210; i++) {
+            body.append((char) ('a' + i % 26));
+        }
+
+        ByteChunk bc = new ByteChunk();
+        Map<String,List<String>> responseHeaders = new HashMap<>();
+        int rc = postUrl(body.toString().getBytes(StandardCharsets.US_ASCII),
+                "http://localhost:" + getPort() + "/test", bc, responseHeaders);
+        bc.setCharset(StandardCharsets.US_ASCII);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+        // One character read, then 20 skipped before the remainder was echoed
+        Assert.assertEquals("20", responseHeaders.get("X-Skip").get(0));
+        Assert.assertEquals(body.substring(21), bc.toString());
+    }
+
+
     private void doUtf8BodyTest(String description, int[] input, String expected) throws Exception {
 
         byte[] bytes = new byte[input.length];
@@ -159,6 +192,34 @@ public class TestInputBuffer extends TomcatBaseTest {
     }
 
 
+    private static class SkipServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+            Reader r = req.getReader();
+            StringBuilder result = new StringBuilder();
+
+            // Consume one character so the skip below starts with 9 buffered chars
+            r.read();
+            long skipped = r.skip(20);
+
+            char[] cbuf = new char[80];
+            int n = r.read(cbuf);
+            while (n > 0) {
+                result.append(cbuf, 0, n);
+                n = r.read(cbuf);
+            }
+
+            resp.setHeader("X-Skip", Long.toString(skipped));
+            resp.setContentType("text/plain");
+            resp.setCharacterEncoding("US-ASCII");
+            resp.getWriter().write(result.toString());
+        }
+    }
+
+
     private static class Bug60400Servlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;


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


(tomcat) 03/08: Return 0 rather than -1 for zero-length reads from InputBuffer at end of stream, as required by the InputStream/Reader contract, and avoid the unnecessary blocking fill these requests previously triggered

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0c9837fd065aa4deacf5e7e807473ac0f9e18368
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 10:53:16 2026 +0200

    Return 0 rather than -1 for zero-length reads from InputBuffer at end of stream, as required by the InputStream/Reader contract, and avoid the unnecessary blocking fill these requests previously triggered
---
 .../org/apache/catalina/connector/InputBuffer.java |  8 +++++
 .../apache/catalina/connector/TestInputBuffer.java | 39 ++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java
index 5fc9285b5e..1c50a0781e 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -395,6 +395,10 @@ public class InputBuffer extends Reader implements ByteChunk.ByteInputChannel, A
     public int read(byte[] b, int off, int len) throws IOException {
         throwIfClosed();
 
+        if (len == 0) {
+            return 0;
+        }
+
         if (checkByteBufferEof()) {
             return -1;
         }
@@ -501,6 +505,10 @@ public class InputBuffer extends Reader implements ByteChunk.ByteInputChannel, A
     public int read(char[] cbuf, int off, int len) throws IOException {
         throwIfClosed();
 
+        if (len == 0) {
+            return 0;
+        }
+
         if (checkCharBufferEof()) {
             return -1;
         }
diff --git a/test/org/apache/catalina/connector/TestInputBuffer.java b/test/org/apache/catalina/connector/TestInputBuffer.java
index bb7ce5ce76..2880a68ace 100644
--- a/test/org/apache/catalina/connector/TestInputBuffer.java
+++ b/test/org/apache/catalina/connector/TestInputBuffer.java
@@ -18,6 +18,7 @@ package org.apache.catalina.connector;
 
 import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.Reader;
 import java.io.Writer;
 import java.nio.charset.MalformedInputException;
@@ -135,6 +136,26 @@ public class TestInputBuffer extends TomcatBaseTest {
     }
 
 
+    @Test
+    public void testZeroLengthReadAtEof() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        Context root = tomcat.addContext("", TEMP_DIR);
+        Tomcat.addServlet(root, "ZeroRead", new ZeroReadServlet());
+        root.addServletMapping("/test", "ZeroRead");
+
+        tomcat.start();
+
+        ByteChunk bc = new ByteChunk();
+        Map<String,List<String>> responseHeaders = new HashMap<>();
+        int rc = postUrl("abc".getBytes(StandardCharsets.US_ASCII),
+                "http://localhost:" + getPort() + "/test", bc, responseHeaders);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+        // Per the InputStream contract, a zero-length read returns 0, even at EOF
+        Assert.assertEquals("0", responseHeaders.get("X-Zero").get(0));
+    }
+
+
     private void doUtf8BodyTest(String description, int[] input, String expected) throws Exception {
 
         byte[] bytes = new byte[input.length];
@@ -192,6 +213,24 @@ public class TestInputBuffer extends TomcatBaseTest {
     }
 
 
+    private static class ZeroReadServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+            InputStream is = req.getInputStream();
+            while (is.read() >= 0) {
+                // Drain the body
+            }
+            int zero = is.read(new byte[1], 0, 0);
+
+            resp.setHeader("X-Zero", Integer.toString(zero));
+            resp.getWriter().write("ok");
+        }
+    }
+
+
     private static class SkipServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;


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


(tomcat) 08/08: Merge branch '11.0.x' of git@github.com:apache/tomcat.git into 11.0.x

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 731e38b9c8c400fd521eb6aaac08e0fcf8ee97a5
Merge: 67ceed2eae 9b0f7e4cfe
Author: remm <re...@apache.org>
AuthorDate: Wed Sep 30 11:35:24 2026 +0200

    Merge branch '11.0.x' of git@github.com:apache/tomcat.git into 11.0.x

 .../apache/catalina/ant/AbstractCatalinaTask.java  | 34 +++++++++++++++++-----
 java/org/apache/catalina/ant/DeployTask.java       |  5 +---
 2 files changed, 27 insertions(+), 12 deletions(-)


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


(tomcat) 04/08: Return a snapshot enumeration from Request.getLocales() to prevent ConcurrentModificationExceptions when the locales list is modified while a previously returned enumeration is being iterated

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 7ae6a1150a34c9a88b1e300540d53ca3a264981e
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 10:54:55 2026 +0200

    Return a snapshot enumeration from Request.getLocales() to prevent ConcurrentModificationExceptions when the locales list is modified while a previously returned enumeration is being iterated
---
 java/org/apache/catalina/connector/Request.java | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index 4dfbe0ce16..d17b522151 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1133,7 +1133,9 @@ public class Request implements HttpServletRequest {
         }
 
         if (!locales.isEmpty()) {
-            return Collections.enumeration(locales);
+            // Take a copy to prevent ConcurrentModificationExceptions if the
+            // list is modified while the enumeration is being iterated
+            return Collections.enumeration(new ArrayList<>(locales));
         }
         ArrayList<Locale> results = new ArrayList<>();
         results.add(defaultLocale);


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


(tomcat) 02/08: Record an error in CoyoteWriter.close() when the underlying OutputBuffer close fails, rather than swallowing the IOException silently, so checkError() reflects the failure

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 528a5b08835f9e74ec919e816c7ccfc3c438323e
Author: opencode <re...@apache.org>
AuthorDate: Wed Sep 30 10:51:55 2026 +0200

    Record an error in CoyoteWriter.close() when the underlying OutputBuffer close fails, rather than swallowing the IOException silently, so checkError() reflects the failure
---
 java/org/apache/catalina/connector/CoyoteWriter.java | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/connector/CoyoteWriter.java b/java/org/apache/catalina/connector/CoyoteWriter.java
index 0cfc488aa8..06a41338ed 100644
--- a/java/org/apache/catalina/connector/CoyoteWriter.java
+++ b/java/org/apache/catalina/connector/CoyoteWriter.java
@@ -111,12 +111,16 @@ public class CoyoteWriter extends PrintWriter {
     @Override
     public void close() {
 
+        if (error) {
+            return;
+        }
+
         // We don't close the PrintWriter - super() is not called,
         // so the stream can be reused. We close ob.
         try {
             ob.close();
-        } catch (IOException ignore) {
-            // Ignore
+        } catch (IOException ioe) {
+            setErrorException(ioe);
         }
 
     }


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