Issue #11387: Reintroduce MultiPartCompliance.LEGACY in ee9/ee8

jetty-core/jetty-http/pom.xml

@@ -28,12 +28,17 @@
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
-
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-slf4j-impl</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty.tests</groupId>
+      <artifactId>jetty-test-multipart</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty.toolchain</groupId>
       <artifactId>jetty-test-helper</artifactId>

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

@@ -1186,6 +1186,7 @@ private boolean parsePreamble(ByteBuffer buffer)
 
         private State parseHeaderStart(ByteBuffer buffer)
         {
+            boolean crFlag = false;
             while (buffer.hasRemaining())
             {
                 HttpTokens.Token token = next(buffer);
@@ -1194,9 +1195,12 @@ private State parseHeaderStart(ByteBuffer buffer)
                     case CR ->
                     {
                         // Ignore CR and loop around;
+                        crFlag = true;
                     }
                     case LF ->
                     {
+                        if (!crFlag)
+                            notifyViolation(MultiPartCompliance.Violation.LF_LINE_TERMINATION);
                         // End of fields.
                         notifyPartHeaders();
                         // A part may have an empty content.
@@ -1211,6 +1215,7 @@ private State parseHeaderStart(ByteBuffer buffer)
                     {
                         if (Character.isWhitespace(token.getByte()))
                         {
+                            notifyViolation(MultiPartCompliance.Violation.WHITESPACE_BEFORE_BOUNDARY);
                             if (text.length() == 0)
                                 throw new BadMessageException("invalid leading whitespace before header");
                         }
@@ -1392,6 +1397,7 @@ private boolean parseContent(Content.Chunk chunk)
             partialBoundaryMatch = boundaryFinder.endsWith(buffer);
             if (partialBoundaryMatch > 0)
             {
+                notifyCRContent();
                 int limit = buffer.limit();
                 int sliceLimit = limit - partialBoundaryMatch;
                 // BoundaryFinder is configured to search for '\n--Boundary';
@@ -1541,6 +1547,19 @@ private void notifyFailure(Throwable failure)
             }
         }
 
+        private void notifyViolation(MultiPartCompliance.Violation violation)
+        {
+            try
+            {
+                listener.onViolation(violation);
+            }
+            catch (Throwable x)
+            {
+                if (LOG.isDebugEnabled())
+                    LOG.debug("failure while notifying listener {}", listener, x);
+            }
+        }
+
         /**
          * <p>A listener for events emitted by a {@link Parser}.</p>
          */
@@ -1603,6 +1622,16 @@ default void onComplete()
             default void onFailure(Throwable failure)
             {
             }
+
+            /**
+             * <p>Callback method invoked when the low level parser encounters
+             * a fundamental multipart violation</p>>
+             *
+             * @param violation the violation detected
+             */
+            default void onViolation(MultiPartCompliance.Violation violation)
+            {
+            }
         }
 
         private enum State

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

@@ -17,6 +17,10 @@
 import java.util.EnumSet;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static java.util.EnumSet.allOf;
+import static java.util.EnumSet.noneOf;
 
 /**
  * The compliance mode for MultiPart handling.
@@ -25,7 +29,12 @@ public class MultiPartCompliance implements ComplianceViolation.Mode
 {
     public enum Violation implements ComplianceViolation
     {
-        CONTENT_TRANSFER_ENCODING("https://tools.ietf.org/html/rfc7578#section-4.7", "Content-Transfer-Encoding is deprecated");
+        CONTENT_TRANSFER_ENCODING("https://tools.ietf.org/html/rfc7578#section-4.7", "Content-Transfer-Encoding header is deprecated"),
+        CR_LINE_TERMINATION("https://tools.ietf.org/html/rfc2046#section-4.1.1", "CR only line termination is forbidden"),
+        LF_LINE_TERMINATION("https://tools.ietf.org/html/rfc2046#section-4.1.1", "LF only line termination is forbidden"),
+        WHITESPACE_BEFORE_BOUNDARY("https://tools.ietf.org/html/rfc2046#section-5.1.1", "Whitespace not allowed before boundary"),
+        BASE64_TRANSFER_ENCODING("https://tools.ietf.org/html/rfc7578#section-4.7", "'base64' Content-Transfer-Encoding is deprecated"),
+        QUOTED_PRINTABLE_TRANSFER_ENCODING("https://tools.ietf.org/html/rfc7578#section-4.7", "'quoted-printable' Content-Transfer-Encoding is deprecated");
 
         private final String url;
         private final String description;
@@ -55,10 +64,22 @@ public String getDescription()
         }
     }
 
+    /**
+     * RFC7578 {@code multiPart/form-data} compliant strict parsing.
+     */
     public static final MultiPartCompliance RFC7578 = new MultiPartCompliance(
         "RFC7578", EnumSet.of(Violation.CONTENT_TRANSFER_ENCODING));
 
-    private static final List<MultiPartCompliance> KNOWN_MODES = Arrays.asList(RFC7578);
+    /**
+     * Legacy {@code multiPart/form-data} parsing which is slow, buggy, but forgiving to a fault.
+     * This mode is not recommended for websites on the public internet.
+     * It will accept non-compliant preambles and inconsistent line termination that are in violation of RFC7578.
+     */
+    public static final MultiPartCompliance LEGACY =  new MultiPartCompliance(
+        "LEGACY", EnumSet.complementOf(EnumSet.of(Violation.BASE64_TRANSFER_ENCODING)));
+
+    private static final List<MultiPartCompliance> KNOWN_MODES = Arrays.asList(RFC7578, LEGACY);
+    private static final AtomicInteger __custom = new AtomicInteger();
 
     public static MultiPartCompliance valueOf(String name)
     {
@@ -70,6 +91,85 @@ public static MultiPartCompliance valueOf(String name)
         return null;
     }
 
+    /**
+     * Create compliance set from a set of allowed Violations.
+     *
+     * @param violations A string of violations to allow:
+     * @return the compliance from the string spec
+     */
+    public static MultiPartCompliance from(Set<MultiPartCompliance.Violation> violations)
+    {
+        return new MultiPartCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
+    }
+
+    /**
+     * Create compliance set from string.
+     * <p>
+     * Format: &lt;BASE&gt;[,[-]&lt;violation&gt;]...
+     * </p>
+     * <p>BASE is one of:</p>
+     * <dl>
+     * <dt>0</dt><dd>No {@link MultiPartCompliance.Violation}s</dd>
+     * <dt>*</dt><dd>All {@link MultiPartCompliance.Violation}s</dd>
+     * <dt>&lt;name&gt;</dt><dd>The name of a static instance of MultiPartCompliance (e.g. {@link MultiPartCompliance#LEGACY}).
+     * </dl>
+     * <p>
+     * The remainder of the list can contain then names of {@link MultiPartCompliance.Violation}s to include them in the mode, or prefixed
+     * with a '-' to exclude them from the mode.  Examples are:
+     * </p>
+     * <dl>
+     * <dt>{@code 0,CONTENT_TRANSFER_ENCODING}</dt><dd>Only allow {@link MultiPartCompliance.Violation#CONTENT_TRANSFER_ENCODING}</dd>
+     * <dt>{@code *,-BASE64_TRANSFER_ENCODING}</dt><dd>Only all except {@link MultiPartCompliance.Violation#BASE64_TRANSFER_ENCODING}</dd>
+     * <dt>{@code LEGACY,BASE64_TRANSFER_ENCODING}</dt><dd>Same as LEGACY plus {@link MultiPartCompliance.Violation#BASE64_TRANSFER_ENCODING}</dd>
+     * </dl>
+     *
+     * @param spec A string describing the compliance
+     * @return the MultiPartCompliance instance derived from the string description
+     */
+    public static MultiPartCompliance from(String spec)
+    {
+        MultiPartCompliance compliance = valueOf(spec);
+        if (compliance == null)
+        {
+            String[] elements = spec.split("\\s*,\\s*");
+
+            Set<MultiPartCompliance.Violation> violations = switch (elements[0])
+            {
+                case "0" -> noneOf(MultiPartCompliance.Violation.class);
+                case "*" -> allOf(MultiPartCompliance.Violation.class);
+                default ->
+                {
+                    MultiPartCompliance mode = MultiPartCompliance.valueOf(elements[0]);
+                    yield (mode == null) ? noneOf(MultiPartCompliance.Violation.class) : copyOf(mode.getAllowed());
+                }
+            };
+
+            for (int i = 1; i < elements.length; i++)
+            {
+                String element = elements[i];
+                boolean exclude = element.startsWith("-");
+                if (exclude)
+                    element = element.substring(1);
+
+                MultiPartCompliance.Violation section = MultiPartCompliance.Violation.valueOf(element);
+                if (exclude)
+                    violations.remove(section);
+                else
+                    violations.add(section);
+            }
+
+            compliance = new MultiPartCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
+        }
+        return compliance;
+    }
+
+    private static Set<MultiPartCompliance.Violation> copyOf(Set<MultiPartCompliance.Violation> violations)
+    {
+        if (violations == null || violations.isEmpty())
+            return EnumSet.noneOf(MultiPartCompliance.Violation.class);
+        return EnumSet.copyOf(violations);
+    }
+
     private final String name;
     private final Set<Violation> violations;
 
@@ -92,13 +192,13 @@ public boolean allows(ComplianceViolation violation)
     }
 
     @Override
-    public Set<? extends ComplianceViolation> getKnown()
+    public Set<Violation> getKnown()
     {
         return EnumSet.allOf(Violation.class);
     }
 
     @Override
-    public Set<? extends ComplianceViolation> getAllowed()
+    public Set<Violation> getAllowed()
     {
         return violations;
     }

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

@@ -283,6 +283,15 @@ public Charset getDefaultCharset()
             return listener.getDefaultCharset();
         }
 
+        /**
+         * Set the default Charset to use if the part {@code _charset_} is missing.
+         * @param charset the charset to use, instead of null.
+         */
+        public void setDefaultCharset(Charset charset)
+        {
+            listener.setDefaultCharset(charset);
+        }
+
         /**
          * @return the max length of a {@link MultiPart.Part} headers, in bytes, or -1 for unlimited length
          */
@@ -430,6 +439,7 @@ private class PartsListener extends MultiPart.AbstractPartsListener
             private final List<Content.Chunk> partChunks = new ArrayList<>();
             private long fileSize;
             private long memoryFileSize;
+            private Charset defaultCharset;
             private Path filePath;
             private SeekableByteChannel fileChannel;
             private Throwable failure;
@@ -536,8 +546,21 @@ public void onPart(String name, String fileName, HttpFields headers)
                     if (headers.contains("content-transfer-encoding"))
                     {
                         String value = headers.get("content-transfer-encoding");
+                        if ("base64".equals(value))
+                            complianceViolationListener.onComplianceViolation(
+                                new ComplianceViolation.Event(MultiPartCompliance.RFC7578,
+                                    MultiPartCompliance.Violation.BASE64_TRANSFER_ENCODING,
+                                    value));
+                        if ("quoted-printable".equals(value))
+                            complianceViolationListener.onComplianceViolation(
+                                new ComplianceViolation.Event(MultiPartCompliance.RFC7578,
+                                    MultiPartCompliance.Violation.QUOTED_PRINTABLE_TRANSFER_ENCODING,
+                                    value));
                         if (!"8bit".equalsIgnoreCase(value) && !"binary".equalsIgnoreCase(value))
-                            complianceViolationListener.onComplianceViolation(new ComplianceViolation.Event(MultiPartCompliance.RFC7578, MultiPartCompliance.Violation.CONTENT_TRANSFER_ENCODING, value));
+                            complianceViolationListener.onComplianceViolation(
+                                new ComplianceViolation.Event(MultiPartCompliance.RFC7578,
+                                    MultiPartCompliance.Violation.CONTENT_TRANSFER_ENCODING,
+                                    value));
                     }
 
                     MultiPart.Part part;
@@ -576,10 +599,15 @@ Charset getDefaultCharset()
                         .map(part -> part.getContentAsString(US_ASCII))
                         .map(Charset::forName)
                         .findFirst()
-                        .orElse(null);
+                        .orElse(defaultCharset);
                 }
             }
 
+            void setDefaultCharset(Charset charset)
+            {
+                defaultCharset = charset;
+            }
+
             int getPartsSize()
             {
                 try (AutoLock ignored = lock.lock())
@@ -594,6 +622,14 @@ public void onFailure(Throwable failure)
                 fail(failure);
             }
 
+            @Override
+            public void onViolation(MultiPartCompliance.Violation violation)
+            {
+                complianceViolationListener.onComplianceViolation(new ComplianceViolation.Event(
+                    MultiPartCompliance.RFC7578, violation, "multipart spec violation"
+                ));
+            }
+
             private void fail(Throwable cause)
             {
                 List<MultiPart.Part> partsToFail;