MultipartEntityBuilder to use a fixed boundary by a default (#619)

Author: arturobernalg

httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/MultipartEntityBuilder.java

@@ -29,40 +29,53 @@
 
 import java.io.File;
 import java.io.InputStream;
-import java.nio.CharBuffer;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.concurrent.ThreadLocalRandom;
+import java.util.UUID;
 
 import org.apache.hc.core5.http.ContentType;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.NameValuePair;
 import org.apache.hc.core5.http.message.BasicNameValuePair;
 import org.apache.hc.core5.util.Args;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Builder for multipart {@link HttpEntity}s.
+ * <p>
+ * This class constructs multipart entities with a boundary determined by either a fixed
+ * value ("httpclient_boundary_7k9p2m4x8n5j3q6t1r0vwyzabcdefghi") or a random UUID. If no
+ * boundary is explicitly set via {@link #setBoundary(String)}, it defaults to the fixed
+ * value unless {@link #withRandomBoundary()} is called to request a random UUID at build
+ * time. Users can provide a custom boundary with {@link #setBoundary(String)}. A warning
+ * is logged when no explicit boundary is set via {@link #setBoundary(String)}, encouraging
+ * deliberate choice.
+ * </p>
  *
  * @since 5.0
  */
     public class MultipartEntityBuilder {
 
-    /**
-     * The pool of ASCII chars to be used for generating a multipart boundary.
-     */
-    private final static char[] MULTIPART_CHARS =
-            "-_1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
-                    .toCharArray();
-
     private ContentType contentType;
     private HttpMultipartMode mode = HttpMultipartMode.STRICT;
     private String boundary;
     private Charset charset;
     private List<MultipartPart> multipartParts;
 
+
+    private static final String BOUNDARY_PREFIX = "httpclient_boundary_";
+
+    private boolean isRandomBoundaryRequested = false;
+    /**
+     * The logger for this class.
+     */
+    private static final Logger LOG = LoggerFactory.getLogger(MultipartEntityBuilder.class);
+
+
     /**
      * The preamble of the multipart message.
      * This field stores the optional preamble that should be added at the beginning of the multipart message.
@@ -104,6 +117,17 @@ public MultipartEntityBuilder setStrictMode() {
         return this;
     }
 
+    /**
+     * Sets a custom boundary string for the multipart entity.
+     * <p>
+     * If {@code null} is provided, the builder reverts to its default boundary logic:
+     * either using a boundary from the {@code contentType} if present, or falling back
+     * to a fixed or random boundary (depending on {@link #withRandomBoundary()}).
+     * </p>
+     *
+     * @param boundary the boundary string, or {@code null} to use the default boundary logic
+     * @return this builder instance
+     */
     public MultipartEntityBuilder setBoundary(final String boundary) {
         this.boundary = boundary;
         return this;
@@ -204,6 +228,20 @@ public MultipartEntityBuilder addBinaryBody(final String name, final InputStream
         return addBinaryBody(name, stream, ContentType.DEFAULT_BINARY, null);
     }
 
+    /**
+     * Returns the fixed default boundary value.
+     */
+    private String getFixedBoundary() {
+        return BOUNDARY_PREFIX + "7k9p2m4x8n5j3q6t1r0vwyzabcdefghi";
+    }
+
+    /**
+     * Generates a random boundary using UUID.
+     */
+    private String getRandomBoundary() {
+        return BOUNDARY_PREFIX + UUID.randomUUID();
+    }
+
     /**
      * Adds a preamble to the multipart entity being constructed. The preamble is the text that appears before the first
      * boundary delimiter. The preamble is optional and may be null.
@@ -231,15 +269,17 @@ public MultipartEntityBuilder addEpilogue(final String epilogue) {
         return this;
     }
 
-    private String generateBoundary() {
-        final ThreadLocalRandom rand = ThreadLocalRandom.current();
-        final int count = rand.nextInt(30, 41); // a random size from 30 to 40
-        final CharBuffer buffer = CharBuffer.allocate(count);
-        while (buffer.hasRemaining()) {
-            buffer.put(MULTIPART_CHARS[rand.nextInt(MULTIPART_CHARS.length)]);
-        }
-        buffer.flip();
-        return buffer.toString();
+    /**
+     * Configures the builder to request a random boundary generated by UUID.randomUUID()
+     * at build time if no explicit boundary is set via {@link #setBoundary(String)}.
+     *
+     * @return this builder instance
+     * @since 5.5
+     */
+    public MultipartEntityBuilder withRandomBoundary() {
+        this.isRandomBoundaryRequested = true;
+        this.boundary = null;
+        return this;
     }
 
     MultipartFormEntity buildEntity() {
@@ -248,7 +288,10 @@ MultipartFormEntity buildEntity() {
             boundaryCopy = contentType.getParameter("boundary");
         }
         if (boundaryCopy == null) {
-            boundaryCopy = generateBoundary();
+            boundaryCopy = isRandomBoundaryRequested ? getRandomBoundary() : getFixedBoundary();
+            if (LOG.isWarnEnabled()) {
+                LOG.warn("No boundary explicitly set; using generated default: {}", boundaryCopy);
+            }
         }
         Charset charsetCopy = charset;
         if (charsetCopy == null && contentType != null) {

httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java

@@ -35,8 +35,11 @@
 import java.util.List;
 
 import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.HeaderElement;
 import org.apache.hc.core5.http.NameValuePair;
+import org.apache.hc.core5.http.message.BasicHeaderValueParser;
 import org.apache.hc.core5.http.message.BasicNameValuePair;
+import org.apache.hc.core5.http.message.ParserCursor;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -307,4 +310,36 @@ void testMultipartWriteToRFC7578ModeWithFilenameStar() throws Exception {
                 "--xxxxxxxxxxxxxxxxxxxxxxxx--\r\n", out.toString(StandardCharsets.ISO_8859_1.name()));
     }
 
+    @Test
+    void testRandomBoundary() {
+        final MultipartFormEntity entity = MultipartEntityBuilder.create()
+                .withRandomBoundary()
+                .buildEntity();
+        final NameValuePair boundaryParam = extractBoundary(entity.getContentType());
+        final String boundary = boundaryParam.getValue();
+        Assertions.assertNotNull(boundary);
+        Assertions.assertEquals(56, boundary.length());
+        Assertions.assertTrue(boundary.startsWith("httpclient_boundary_"));
+        Assertions.assertTrue(boundary.substring(20).matches("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"));
+    }
+
+    @Test
+    void testExplicitBoundaryOverridesRandom() {
+        final String customBoundary = "my_custom_boundary";
+        final MultipartFormEntity entity = MultipartEntityBuilder.create()
+                .withRandomBoundary()
+                .setBoundary(customBoundary)
+                .buildEntity();
+        final NameValuePair boundaryParam = extractBoundary(entity.getContentType());
+        Assertions.assertEquals(customBoundary, boundaryParam.getValue());
+    }
+
+    private NameValuePair extractBoundary(final String contentType) {
+        final BasicHeaderValueParser parser = BasicHeaderValueParser.INSTANCE;
+        final ParserCursor cursor = new ParserCursor(0, contentType.length());
+        final HeaderElement elem = parser.parseHeaderElement(contentType, cursor);
+        Assertions.assertEquals("multipart/mixed", elem.getName());
+        return elem.getParameterByName("boundary");
+    }
+
 }

httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartFormHttpEntity.java

@@ -78,9 +78,7 @@ void testImplicitContractorParams() {
         final String boundary = p1.getValue();
         Assertions.assertNotNull(boundary);
 
-        Assertions.assertTrue(boundary.length() >= 30);
-        Assertions.assertTrue(boundary.length() <= 40);
-
+        Assertions.assertEquals(52, boundary.length());
         final NameValuePair p2 = elem.getParameterByName("charset");
         Assertions.assertNull(p2);
     }