- * 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. + * This class constructs multipart entities with a boundary determined by either a random UUID + * or an explicit boundary set via {@link #setBoundary(String)}. *
*- * IMPORTANT: it is responsibility of the caller to validate / sanitize content of body - * parts, for instance, to ensure they do not contain the boundary value that can prevent - * the consumer of the entity from correctly parsing / processing the body parts. + * IMPORTANT: it is responsibility of the caller to validate / sanitize content of body + * parts. For instance, when using an explicit boundary, it's the caller's responsibility to + * ensure the body parts do not contain the boundary value, which can prevent the consumer of + * the entity from correctly parsing / processing the body parts. *
* * @since 5.0 @@ -74,7 +70,6 @@ public class MultipartEntityBuilder { private static final String BOUNDARY_PREFIX = "httpclient_boundary_"; - private boolean isRandomBoundaryRequested = false; /** * The logger for this class. */ @@ -125,12 +120,14 @@ public MultipartEntityBuilder setStrictMode() { /** * Sets a custom boundary string for the multipart entity. *- * 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()}). + * If {@code null} is provided, the builder reverts to its default logic of using a random UUID. + *
+ *+ * IMPORTANT: when setting an explicit boundary, it is responsibility of the caller to validate / sanitize content + * of body parts to ensure they do not contain the boundary value. *
* - * @param boundary the boundary string, or {@code null} to use the default boundary logic + * @param boundary the boundary string, or {@code null} to use a random UUID. * @return this builder instance */ public MultipartEntityBuilder setBoundary(final String boundary) { @@ -234,14 +231,12 @@ public MultipartEntityBuilder addBinaryBody(final String name, final InputStream } /** - * Returns the fixed default boundary value. - */ - private String getFixedBoundary() { - return BOUNDARY_PREFIX + "7k9p2m4x8n5j3q6t1r0vwyzabcdefghi"; - } - - /** - * Generates a random boundary using UUID. + * Generates a random boundary using UUID. The UUID is a v4 random UUID generated from a cryptographically-secure + * random source. + *+ * A cryptographically-secure random number source is used to generate the UUID, to avoid a malicious actor crafting + * a body part that contains the boundary value to tamper with the entity structure. + *
*/ private String getRandomBoundary() { return BOUNDARY_PREFIX + UUID.randomUUID(); @@ -274,29 +269,13 @@ public MultipartEntityBuilder addEpilogue(final String epilogue) { return this; } - /** - * 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() { String boundaryCopy = boundary; if (boundaryCopy == null && contentType != null) { boundaryCopy = contentType.getParameter("boundary"); } if (boundaryCopy == null) { - boundaryCopy = isRandomBoundaryRequested ? getRandomBoundary() : getFixedBoundary(); - if (LOG.isWarnEnabled()) { - LOG.warn("No boundary explicitly set; using generated default: {}", boundaryCopy); - } + boundaryCopy = getRandomBoundary(); } Charset charsetCopy = charset; if (charsetCopy == null && contentType != null) { diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java index 5cbc878881..79e00e39e3 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartEntityBuilder.java @@ -313,9 +313,8 @@ void testMultipartWriteToRFC7578ModeWithFilenameStar() throws Exception { @Test void testRandomBoundary() { final MultipartFormEntity entity = MultipartEntityBuilder.create() - .withRandomBoundary() .buildEntity(); - final NameValuePair boundaryParam = extractBoundary(entity.getContentType()); + final NameValuePair boundaryParam = extractBoundary(entity.getContentType(), "multipart/mixed"); final String boundary = boundaryParam.getValue(); Assertions.assertNotNull(boundary); Assertions.assertEquals(56, boundary.length()); @@ -324,21 +323,43 @@ void testRandomBoundary() { } @Test - void testExplicitBoundaryOverridesRandom() { - final String customBoundary = "my_custom_boundary"; + void testRandomBoundaryWriteTo() throws Exception { + final String helloWorld = "hello world"; + final List