Experiment with a fully async ContentSourceCompletableFuture

documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java

@@ -216,13 +216,11 @@ public static void testFutureString() throws Exception
         Content.Sink.write(source, true, "Three", writeCallback);
         if (!writeCallback.isDone() || !future.isDone())
             throw new IllegalStateException("Should be consumed");
-
-        System.err.println(future.get());
     }
 
     public static class FutureUtf8String extends ContentSourceCompletableFuture<String>
     {
-        Utf8StringBuilder builder = new Utf8StringBuilder();
+        private final Utf8StringBuilder builder = new Utf8StringBuilder();
 
         public FutureUtf8String(Content.Source content)
         {

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartByteRanges.java

@@ -52,7 +52,7 @@
  *
  * @see Parts
  */
-public class MultiPartByteRanges extends CompletableFuture<MultiPartByteRanges.Parts>
+public class MultiPartByteRanges
 {
     private MultiPartByteRanges()
     {

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPartFormData.java

@@ -224,7 +224,7 @@ protected Parts parse(Content.Chunk chunk) throws Throwable
                         throw listener.failure;
                     length += chunk.getByteBuffer().remaining();
                     long max = getMaxLength();
-                    if (max > 0 && length > max)
+                    if (max >= 0 && length > max)
                         throw new IllegalStateException("max length exceeded: %d".formatted(max));
                     parser.parse(chunk);
                     if (listener.isFailed())
@@ -394,7 +394,7 @@ int getPartsSize()
 
         private class PartsListener extends MultiPart.AbstractPartsListener
         {
-            private final AutoLock lock = new AutoLock(); // TODO why do we need this lock?
+            private final AutoLock lock = new AutoLock();
             private final List<MultiPart.Part> parts = new ArrayList<>();
             private final List<Content.Chunk> partChunks = new ArrayList<>();
             private long fileSize;

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java

@@ -24,8 +24,8 @@
  * <p>
  * An example usage to asynchronously read UTF-8 content is:
  * </p>
- * <pre>
- *     public static class FutureUtf8String extends ContentSourceCompletableFuture&lt;String&gt;
+ * <pre>{@code
+ *     public static class FutureUtf8String extends ContentSourceCompletableFuture<String>;
  *     {
  *         Utf8StringBuilder builder = new Utf8StringBuilder();
  *
@@ -34,7 +34,7 @@
  *             super(content);
  *         }
  *
- *         &#064;Override
+ *         @Override
  *         protected String parse(Content.Chunk chunk) throws Throwable
  *         {
  *             if (chunk.hasRemaining())
@@ -43,10 +43,8 @@
  *         }
  *     }
  *     ...
- *     {
- *         new FutureUtf8String(source).thenAccept(System.err::println);
- *     }
- * </pre>
+ *     new FutureUtf8String(source).thenAccept(System.err::println);
+ * }</pre>
  */
 public abstract class ContentSourceCompletableFuture<X> extends CompletableFuture<X>
 {
@@ -57,25 +55,27 @@ public ContentSourceCompletableFuture(Content.Source content)
         _content = content;
     }
 
-    public CompletableFuture<X> parse()
-    {
-        onContentAvailable();
-        return this;
-    }
-
-    private void onContentAvailable()
+    /**
+     * Progress the parsing, {@link Content.Source#read() reading} and/or {@link Content.Source#demand(Runnable) demanding}
+     * as necessary.
+     * <p>
+     * This method must be called once to initiate the reading and parsing,
+     * and is then called to progress parsing in response to any {@link Content.Source#demand(Runnable) demand} calls.
+     * </p>
+     */
+    public void parse()
     {
         while (true)
         {
             Content.Chunk chunk = _content.read();
             if (chunk == null)
             {
-                _content.demand(this::onContentAvailable);
+                _content.demand(this::parse);
                 return;
             }
             if (Content.Chunk.isFailure(chunk))
             {
-                if (!chunk.isLast() && ignoreTransientFailure(chunk.getFailure()))
+                if (!chunk.isLast() && onTransientFailure(chunk.getFailure()))
                     continue;
                 completeExceptionally(chunk.getFailure());
                 return;
@@ -110,9 +110,11 @@ private void onContentAvailable()
 
     /**
      * Called to parse a {@link org.eclipse.jetty.io.Content.Chunk}
-     * @param chunk The chunk containing content to parse. The chunk will
-     * never be a
-     * {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}.
+     * @param chunk The chunk containing content to parse. The chunk will never be null nor a
+     *              {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}.
+     *              If references to the content of the chunk are to be held beyond the scope of this call,
+     *              then implementations must call {@link Content.Chunk#retain()} and {@link Content.Chunk#release()}
+     *              as appropriate.
      * @return The parsed {@code X} instance or null if parsing is not yet complete
      * @throws Throwable Thrown if there is an error parsing
      */
@@ -123,7 +125,7 @@ private void onContentAvailable()
      *             {@link org.eclipse.jetty.io.Content.Chunk#isFailure(Content.Chunk) failure chunk}
      * @return True if the chunk can be ignored.
      */
-    protected boolean ignoreTransientFailure(Throwable cause)
+    protected boolean onTransientFailure(Throwable cause)
     {
         return false;
     }

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/FormFields.java

@@ -59,43 +59,102 @@ public static Charset getFormEncodedCharset(Request request)
         return StringUtil.isEmpty(cs) ? StandardCharsets.UTF_8 : Charset.forName(cs);
     }
 
+    /**
+     * Set a {@link Fields} or related failure for the request
+     * @param request The request to which to associate the fields with
+     * @param fields A {@link CompletableFuture} that will provide either the fields or a failure.
+     */
+    public static void set(Request request, CompletableFuture<Fields> fields)
+    {
+        request.setAttribute(FormFields.class.getName(), fields);
+    }
+
+    /**
+     * @param request The request to enquire from
+     * @return A {@link CompletableFuture} that will provide either the fields or a failure, or null if none set.
+     * @see #from(Request)
+     *
+     */
+    public static CompletableFuture<Fields> get(Request request)
+    {
+        Object attr = request.getAttribute(FormFields.class.getName());
+        if (attr instanceof FormFields futureFormFields)
+            return futureFormFields;
+        return EMPTY;
+    }
+
+    /**
+     * Find or create a {@link FormFields} from a {@link Content.Source}.
+     * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute,
+     *                using the classname as the attribute name, else the request is used
+     *                as a {@link Content.Source} from which to read the fields and set the attribute.
+     * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure.
+     * @see #from(Content.Source, Attributes, Charset, int, int)
+     */
     public static CompletableFuture<Fields> from(Request request)
     {
         int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE);
         int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE);
         return from(request, maxFields, maxLength);
     }
 
+    /**
+     * Find or create a {@link FormFields} from a {@link Content.Source}.
+     * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute,
+     *                using the classname as the attribute name, else the request is used
+     *                as a {@link Content.Source} from which to read the fields and set the attribute.
+     * @param charset the {@link Charset} to use for byte to string conversion.
+     * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure.
+     * @see #from(Content.Source, Attributes, Charset, int, int)
+     */
     public static CompletableFuture<Fields> from(Request request, Charset charset)
     {
         int maxFields = getRequestAttribute(request, FormFields.MAX_FIELDS_ATTRIBUTE);
         int maxLength = getRequestAttribute(request, FormFields.MAX_LENGTH_ATTRIBUTE);
         return from(request, charset, maxFields, maxLength);
     }
 
-    public static void set(Request request, CompletableFuture<Fields> fields)
-    {
-        request.setAttribute(FormFields.class.getName(), fields);
-    }
-
-    public static CompletableFuture<Fields> get(Request request)
-    {
-        Object attr = request.getAttribute(FormFields.class.getName());
-        if (attr instanceof FormFields futureFormFields)
-            return futureFormFields;
-        return EMPTY;
-    }
-
+    /**
+     * Find or create a {@link FormFields} from a {@link Content.Source}.
+     * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute,
+     *                using the classname as the attribute name, else the request is used
+     *                as a {@link Content.Source} from which to read the fields and set the attribute.
+     * @param maxFields The maximum number of fields to be parsed
+     * @param maxLength The maximum total size of the fields
+     * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure.
+     * @see #from(Content.Source, Attributes, Charset, int, int)
+     */
     public static CompletableFuture<Fields> from(Request request, int maxFields, int maxLength)
     {
         return from(request, getFormEncodedCharset(request), maxFields, maxLength);
     }
 
+    /**
+     * Find or create a {@link FormFields} from a {@link Content.Source}.
+     * @param request The {@link Request} in which to look for an existing {@link FormFields} attribute,
+     *                using the classname as the attribute name, else the request is used
+     *                as a {@link Content.Source} from which to read the fields and set the attribute.
+     * @param charset the {@link Charset} to use for byte to string conversion.
+     * @param maxFields The maximum number of fields to be parsed
+     * @param maxLength The maximum total size of the fields
+     * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure.
+     * @see #from(Content.Source, Attributes, Charset, int, int)
+     */
     public static CompletableFuture<Fields> from(Request request, Charset charset, int maxFields, int maxLength)
     {
         return from(request, request, charset, maxFields, maxLength);
     }
 
+    /**
+     * Find or create a {@link FormFields} from a {@link Content.Source}.
+     * @param source The {@link Content.Source} from which to read the fields.
+     * @param attributes The {@link Attributes} in which to look for an existing {@link FormFields}, else in which to
+     *                   add the fields, using the classname as the attribute name.
+     * @param charset the {@link Charset} to use for byte to string conversion.
+     * @param maxFields The maximum number of fields to be parsed
+     * @param maxLength The maximum total size of the fields
+     * @return A {@link CompletableFuture} that will provide the {@link Fields} or a failure.
+     */
     static CompletableFuture<Fields> from(Content.Source source, Attributes attributes, Charset charset, int maxFields, int maxLength)
     {
         Object attr = attributes.getAttribute(FormFields.class.getName());
@@ -128,7 +187,6 @@ private static int getRequestAttribute(Request request, String attribute)
         }
     }
 
-    private final Content.Source _source;
     private final Fields _fields;
     private final CharsetStringBuilder _builder;
     private final int _maxFields;
@@ -138,10 +196,9 @@ private static int getRequestAttribute(Request request, String attribute)
     private int _percent = 0;
     private byte _percentCode;
 
-    public FormFields(Content.Source source, Charset charset, int maxFields, int maxSize)
+    private FormFields(Content.Source source, Charset charset, int maxFields, int maxSize)
     {
         super(source);
-        _source = source;
         _maxFields = maxFields;
         _maxLength = maxSize;
         _builder = CharsetStringBuilder.forCharset(charset);
@@ -225,7 +282,7 @@ protected Fields parse(Content.Chunk chunk) throws CharacterCodingException
                     Fields.Field field = new Fields.Field(_name, value);
                     _name = null;
                     value = null;
-                    if (_maxFields > 0 && _fields.getSize() >= _maxFields)
+                    if (_maxFields >= 0 && _fields.getSize() >= _maxFields)
                         throw new IllegalStateException("form with too many fields > " + _maxFields);
                     _fields.add(field);
                 }

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java

@@ -25,6 +25,7 @@
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.Fields;
+import org.eclipse.jetty.util.FutureCallback;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -61,9 +62,11 @@ public void testFormFields(List<String> chunks, Charset charset, int maxFields,
         assertFalse(futureFields.isDone());
 
         int last = chunks.size() - 1;
+        FutureCallback eof = new FutureCallback();
         for (int i = 0; i <= last; i++)
-            source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), Callback.NOOP);
+            source.write(i == last, BufferUtil.toBuffer(chunks.get(i), charset), i == last ? eof : Callback.NOOP);
 
+        assertTrue(eof.isDone());
         assertTrue(futureFields.isDone());
 
         try

jetty-ee10/jetty-ee10-quickstart/src/main/java/org/eclipse/jetty/ee10/quickstart/QuickStartGeneratorConfiguration.java

@@ -764,7 +764,7 @@ private void outholder(XmlAppendable out, MetaData md, ServletHolder holder) thr
         }
 
         //multipart-config
-        MultipartConfigElement multipartConfig = ((ServletHolder.Registration)holder.getRegistration()).getMultipartConfig();
+        MultipartConfigElement multipartConfig = holder.getRegistration().getMultipartConfigElement();
         if (multipartConfig != null)
         {
             out.openTag("multipart-config", origin(md, holder.getName() + ".servlet.multipart-config"));

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/Dispatcher.java

@@ -309,6 +309,15 @@ public Object getAttribute(String name)
                 {
                     return null;
                 }
+                case ServletContextRequest.MULTIPART_CONFIG_ELEMENT ->
+                {
+                    // If we already have future parts, return the configuration of the wrapped request.
+                    if (super.getAttribute(ServletMultiPartFormData.class.getName()) != null)
+                        return super.getAttribute(name);
+                    // otherwise, return the configuration of this mapping
+                    return _mappedServlet.getServletHolder().getMultipartConfigElement();
+                }
+
                 default ->
                 {
                     return super.getAttribute(name);

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/PreloadFormHandler.java → jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/EagerFormHandler.java

@@ -24,20 +24,20 @@
 import org.eclipse.jetty.util.Callback;
 
 /**
- * Handler to asynchronously preload & parse {@link MimeTypes.Type#FORM_ENCODED} and
+ * Handler to eagerly and asynchronously read and parse {@link MimeTypes.Type#FORM_ENCODED} and
  * {@link MimeTypes.Type#MULTIPART_FORM_DATA} content prior to invoking the {@link ServletHandler},
  * which can then consume them with blocking APIs but without blocking.
  * @see FormFields#from(Request)
  * @see ServletMultiPartFormData#from(ServletRequest)
  */
-public class PreloadFormHandler extends Handler.Wrapper
+public class EagerFormHandler extends Handler.Wrapper
 {
-    public PreloadFormHandler()
+    public EagerFormHandler()
     {
         this(null);
     }
 
-    public PreloadFormHandler(Handler handler)
+    public EagerFormHandler(Handler handler)
     {
         super(handler);
     }
@@ -63,15 +63,11 @@ public boolean handle(Request request, org.eclipse.jetty.server.Response respons
         if (future == null)
             return super.handle(request, response, callback);
 
-        if (future.isDone())
-        {
-            if (!super.handle(request, response, callback))
-                callback.failed(new IllegalStateException("Not Handled"));
-            return true;
-        }
-
         future.whenComplete((result, failure) ->
         {
+            // The result and failure are not handled here. Rather we call the next handler
+            // to allow the normal processing to handle the result or failure, which will be
+            // provided via the attribute to ServletApiRequest#getParts()
             try
             {
                 if (!super.handle(request, response, callback))

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java

@@ -240,7 +240,7 @@ public Object getAttribute(String name)
             case "jakarta.servlet.request.key_size" -> super.getAttribute(SecureRequestCustomizer.KEY_SIZE_ATTRIBUTE);
             case "jakarta.servlet.request.ssl_session_id" -> super.getAttribute(SecureRequestCustomizer.SSL_SESSION_ID_ATTRIBUTE);
             case "jakarta.servlet.request.X509Certificate" -> super.getAttribute(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE);
-            case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfig();
+            case ServletContextRequest.MULTIPART_CONFIG_ELEMENT -> _matchedResource.getResource().getServletHolder().getMultipartConfigElement();
             case FormFields.MAX_FIELDS_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormKeys();
             case FormFields.MAX_LENGTH_ATTRIBUTE -> getServletContext().getServletContextHandler().getMaxFormContentSize();
             default -> super.getAttribute(name);
@@ -259,8 +259,12 @@ public Set<String> getAttributeNameSet()
             names.add("jakarta.servlet.request.ssl_session_id");
         if (names.contains(SecureRequestCustomizer.PEER_CERTIFICATES_ATTRIBUTE))
             names.add("jakarta.servlet.request.X509Certificate");
-        if (_matchedResource.getResource().getServletHolder().getMultipartConfig() != null)
+        if (_matchedResource.getResource().getServletHolder().getMultipartConfigElement() != null)
             names.add(ServletContextRequest.MULTIPART_CONFIG_ELEMENT);
+        if (getServletContext().getServletContextHandler().getMaxFormKeys() >= 0)
+            names.add(FormFields.MAX_FIELDS_ATTRIBUTE);
+        if (getServletContext().getServletContextHandler().getMaxFormContentSize() >= 0L)
+            names.add(FormFields.MAX_FIELDS_ATTRIBUTE);
         return names;
     }