You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2025/03/23 09:24:26 UTC
(httpcomponents-client) branch master updated (cd3d69d4b -> 5e62dac13)
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
discard cd3d69d4b Adjust comments
discard 5087f6376 Use a random boundary value by default
new 5e62dac13 MultipartEntityBuilder to use a random boundary value by default
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (cd3d69d4b)
\
N -- N -- N refs/heads/master (5e62dac13)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
(httpcomponents-client) 01/01: MultipartEntityBuilder to use a random boundary value by default
Posted by ol...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
commit 5e62dac13b3075a90c2e84139861356035e2ed31
Author: Ben Shonaldmann <be...@benweissmann.com>
AuthorDate: Mon Mar 17 11:25:05 2025 -0400
MultipartEntityBuilder to use a random boundary value by default
---
.../http/entity/mime/MultipartEntityBuilder.java | 59 +++++++---------------
.../entity/mime/TestMultipartEntityBuilder.java | 41 +++++++++++----
.../entity/mime/TestMultipartFormHttpEntity.java | 3 +-
3 files changed, 52 insertions(+), 51 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/MultipartEntityBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/MultipartEntityBuilder.java
index e3bce018c..de16e98c2 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/MultipartEntityBuilder.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/entity/mime/MultipartEntityBuilder.java
@@ -47,18 +47,14 @@
/**
* 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.
+ * This class constructs multipart entities with a boundary determined by either a random UUID
+ * or an explicit boundary set via {@link #setBoundary(String)}.
* </p>
* <p>
- * 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.
* </p>
*
* @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.
* <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()}).
+ * If {@code null} is provided, the builder reverts to its default logic of using a random UUID.
+ * </p>
+ * <p>
+ * 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.
* </p>
*
- * @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.
+ * <p>
+ * 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.
+ * </p>
*/
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 5cbc87888..79e00e39e 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<NameValuePair> parameters = new ArrayList<>();
+ parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_NAME, "test"));
+ parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_FILENAME, helloWorld));
final MultipartFormEntity entity = MultipartEntityBuilder.create()
- .withRandomBoundary()
- .setBoundary(customBoundary)
+ .setStrictMode()
+ .addPart(new FormBodyPartBuilder()
+ .setName("test")
+ .setBody(new StringBody("hello world", ContentType.TEXT_PLAIN))
+ .addField("Content-Disposition", "multipart/form-data", parameters)
+ .build())
.buildEntity();
- final NameValuePair boundaryParam = extractBoundary(entity.getContentType());
- Assertions.assertEquals(customBoundary, boundaryParam.getValue());
+
+ final NameValuePair boundaryParam = extractBoundary(entity.getContentType(), "multipart/form-data");
+ 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}"));
+
+ final ByteArrayOutputStream out = new ByteArrayOutputStream();
+ entity.writeTo(out);
+ out.close();
+ Assertions.assertEquals("--" + boundary + "\r\n" +
+ "Content-Disposition: multipart/form-data; name=\"test\"; filename=\"hello world\"\r\n" +
+ "Content-Type: text/plain; charset=UTF-8\r\n" +
+ "\r\n" +
+ helloWorld + "\r\n" +
+ "--" + boundary + "--\r\n", out.toString(StandardCharsets.US_ASCII.name()));
}
- private NameValuePair extractBoundary(final String contentType) {
+ private NameValuePair extractBoundary(final String contentType, final String expectedName) {
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());
+ Assertions.assertEquals(expectedName, elem.getName());
return elem.getParameterByName("boundary");
}
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartFormHttpEntity.java b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartFormHttpEntity.java
index 3e4d79b1f..1e5d8ecff 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartFormHttpEntity.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/entity/mime/TestMultipartFormHttpEntity.java
@@ -78,7 +78,8 @@ void testImplicitContractorParams() {
final String boundary = p1.getValue();
Assertions.assertNotNull(boundary);
- Assertions.assertEquals(52, boundary.length());
+ // "httpclient_boundary_" + UUID (32 characters + 4 dashes) + 56 characters
+ Assertions.assertEquals(56, boundary.length());
final NameValuePair p2 = elem.getParameterByName("charset");
Assertions.assertNull(p2);
}