Support limits on JSON length for Jackson form binding too

api/src/org/labkey/api/ApiModule.java

@@ -116,6 +116,7 @@
 import org.labkey.api.reader.ExcelLoader;
 import org.labkey.api.reader.JSONDataLoader;
 import org.labkey.api.reader.MapLoader;
+import org.labkey.api.reader.StrictBoundedReader;
 import org.labkey.api.reader.TabLoader;
 import org.labkey.api.reports.model.ViewCategoryManager;
 import org.labkey.api.reports.report.ReportType;
@@ -439,6 +440,7 @@ public void registerServlets(ServletContext servletCtx)
             SimpleFilter.InClauseTestCase.class,
             SimpleFilter.SqlClauseTestCase.class,
             SqlScanner.TestCase.class,
+            StrictBoundedReader.TestCase.class,
             StringExpressionFactory.TestCase.class,
             StringUtilsLabKey.TestCase.class,
             SubfolderWriter.TestCase.class,

api/src/org/labkey/api/action/BaseApiAction.java

@@ -365,8 +365,11 @@ protected FormAndErrors<FORM> populateJacksonForm() throws Exception
             // Ideally, ObjectReader would handle the Object case as well, but currently readValue() throws with "end-of-input" exception
             if (Object.class != c)
             {
-                ObjectReader reader = getObjectReader(c);
-                form = reader.readValue(getViewContext().getRequest().getInputStream());
+                ObjectReader objectReader = getObjectReader(c);
+                try (Reader requestReader = openRequestReader())
+                {
+                    form = objectReader.readValue(requestReader);
+                }
             }
             else
             {
@@ -385,7 +388,7 @@ protected FormAndErrors<FORM> populateJacksonForm() throws Exception
             errors = new NullSafeBindException(new Object(), "form");
             errors.reject(SpringActionController.ERROR_MSG, "Error binding property: " + x.getMessage());
         }
-        catch (JsonProcessingException x)
+        catch (JsonProcessingException | StrictBoundedReader.LimitExceededException x)
         {
             // Bad JSON
             throw new BadRequestException(x.getMessage(), x);
@@ -516,20 +519,24 @@ protected long getMaximumJsonInputLength()
         if (request == null)
             return null;
 
+        try (Reader reader = openRequestReader())
+        {
+            JSONTokener tokener = new JSONTokener(reader);
+            return tokener.more() ? new JSONObject(tokener) : null;
+        }
+    }
+
+    private Reader openRequestReader() throws IOException
+    {
+        HttpServletRequest request = getViewContext().getRequest();
         String characterEncoding = request.getCharacterEncoding();
         if (characterEncoding == null)
             characterEncoding = StringUtilsLabKey.DEFAULT_CHARSET.name();
-
         long maxLength = getMaximumJsonInputLength();
-
         // Issue 53699: Use request.getInputStream() instead of request.getReader() to
         // avoid BufferUnderflowException when processing multibyte character JSON payloads.
-        try (Reader streamReader = new InputStreamReader(request.getInputStream(), characterEncoding);
-             Reader jsonReader = maxLength > 0 ? new StrictBoundedReader(streamReader, maxLength) : streamReader)
-        {
-            JSONTokener tokener = new JSONTokener(jsonReader);
-            return tokener.more() ? new JSONObject(tokener) : null;
-        }
+        Reader streamReader = new InputStreamReader(request.getInputStream(), characterEncoding);
+        return maxLength > 0 ? new StrictBoundedReader(streamReader, maxLength) : streamReader;
     }
 
     private BindException populateForm(@Nullable JSONObject jsonObj, FORM form)

api/src/org/labkey/api/reader/StrictBoundedReader.java

@@ -1,7 +1,14 @@
 package org.labkey.api.reader;
 
+import org.jetbrains.annotations.NotNull;
+import org.jspecify.annotations.NonNull;
+import org.junit.Assert;
+import org.junit.Test;
+import org.labkey.api.test.TestWhen;
+
 import java.io.IOException;
 import java.io.Reader;
+import java.io.StringReader;
 
 /**
  * Restricts the Reader to a character limit. Throws an exception when the limit is exceeded.
@@ -19,12 +26,13 @@ public static class LimitExceededException extends IOException
         public LimitExceededException(String message)
         {
             super(message);
-
         }
     }
 
-    public StrictBoundedReader(Reader in, long maxChars)
+    @SuppressWarnings("NullableProblems")
+    public StrictBoundedReader(@NotNull Reader in, long maxChars)
     {
+        //noinspection ConstantValue
         if (in == null)
         {
             throw new NullPointerException("Reader cannot be null");
@@ -54,7 +62,7 @@ public int read() throws IOException
     }
 
     @Override
-    public int read(char[] cbuf, int off, int len) throws IOException
+    public int read(char @NonNull [] cbuf, int off, int len) throws IOException
     {
         ensureOpen();
         if (charsRead >= maxChars)
@@ -96,4 +104,151 @@ public void close() throws IOException
             closed = true;
         }
     }
+
+    @TestWhen(TestWhen.When.BVT)
+    public static class TestCase extends Assert
+    {
+        @Test
+        public void testSingleCharReadWithinLimit() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 10))
+            {
+                assertEquals('h', reader.read());
+                assertEquals('e', reader.read());
+                assertEquals('l', reader.read());
+            }
+        }
+
+        @Test
+        public void testBulkReadWithinLimit() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 10))
+            {
+                char[] buf = new char[5];
+                int n = reader.read(buf, 0, 5);
+                assertEquals(5, n);
+                assertEquals("hello", new String(buf, 0, n));
+            }
+        }
+
+        @Test
+        public void testBulkReadTruncatedToLimit() throws IOException
+        {
+            // When requesting more chars than the limit allows, the read is capped at remaining
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello world"), 5))
+            {
+                char[] buf = new char[20];
+                int n = reader.read(buf, 0, 20);
+                assertEquals(5, n);
+                assertEquals("hello", new String(buf, 0, n));
+            }
+        }
+
+        @Test
+        public void testSingleReadThrowsAfterLimitReached() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 3))
+            {
+                assertEquals('h', reader.read());
+                assertEquals('e', reader.read());
+                assertEquals('l', reader.read());
+                try
+                {
+                    var _ = reader.read();
+                    fail("Expected LimitExceededException");
+                }
+                catch (LimitExceededException e)
+                {
+                    // expected
+                }
+            }
+        }
+
+        @Test
+        public void testBulkReadThrowsAfterLimitReached() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello world"), 5))
+            {
+                char[] buf = new char[20];
+                var _ = reader.read(buf, 0, 20);
+                try
+                {
+                    var _ = reader.read(buf, 0, 1);
+                    fail("Expected LimitExceededException");
+                }
+                catch (LimitExceededException e)
+                {
+                    // expected
+                }
+            }
+        }
+
+        @Test
+        public void testZeroLimitThrowsImmediately() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 0))
+            {
+                try
+                {
+                    var _ = reader.read();
+                    fail("Expected LimitExceededException");
+                }
+                catch (LimitExceededException e)
+                {
+                    // expected
+                }
+            }
+        }
+
+        @Test
+        public void testEofReturnedBeforeLimit() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hi"), 100))
+            {
+                assertEquals('h', reader.read());
+                assertEquals('i', reader.read());
+                assertEquals(-1, reader.read());
+            }
+        }
+
+        @Test
+        public void testBulkEofReturnedBeforeLimit() throws IOException
+        {
+            try (StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hi"), 100))
+            {
+                char[] buf = new char[10];
+                assertEquals(2, reader.read(buf, 0, 10));
+                assertEquals(-1, reader.read(buf, 0, 10));
+            }
+        }
+
+        @Test(expected = NullPointerException.class)
+        public void testNullReaderThrows()
+        {
+            //noinspection DataFlowIssue,resource
+            new StrictBoundedReader(null, 10);
+        }
+
+        @Test(expected = IllegalArgumentException.class)
+        public void testNegativeLimitThrows()
+        {
+            new StrictBoundedReader(new StringReader("hello"), -1);
+        }
+
+        @Test(expected = IOException.class)
+        public void testReadAfterCloseThrows() throws IOException
+        {
+            StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 10);
+            reader.close();
+            var _ = reader.read();
+        }
+
+        @Test
+        public void testDoubleCloseIsIdempotent() throws IOException
+        {
+            StrictBoundedReader reader = new StrictBoundedReader(new StringReader("hello"), 10);
+            reader.close();
+            reader.close(); // should not throw
+        }
+    }
 }

devtools/src/org/labkey/devtools/DevtoolsModule.java

@@ -29,8 +29,11 @@
 import org.labkey.devtools.authentication.TestSsoController;
 import org.labkey.devtools.authentication.TestSsoProvider;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
+import java.util.Set;
 import java.util.function.Supplier;
 
 public class DevtoolsModule extends CodeOnlyModule
@@ -75,6 +78,16 @@ public void doStartup(ModuleContext moduleContext)
     @Override
     public @NotNull Collection<Supplier<Class<?>>> getIntegrationTestFactories()
     {
-        return Collections.singletonList(new JspTestCase("/org/labkey/devtools/test/JspTestCaseTest.jsp"));
+        List<Supplier<Class<?>>> list = new ArrayList<>(super.getIntegrationTestFactories());
+        list.add(new JspTestCase("/org/labkey/devtools/test/JspTestCaseTest.jsp"));
+        return list;
+    }
+
+    @Override
+    public @NotNull Set<Class<?>> getIntegrationTests()
+    {
+        return Set.of(
+            TestController.JsonInputLimitTest.class
+        );
     }
 }