Skip to content

Commit 5087f63

Browse files
benweissmannok2c
authored andcommitted
Use a random boundary value by default
1 parent 57d5a13 commit 5087f63

3 files changed

Lines changed: 48 additions & 53 deletions

File tree

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

Lines changed: 15 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,8 @@
4747
/**
4848
* Builder for multipart {@link HttpEntity}s.
4949
* <p>
50-
* This class constructs multipart entities with a boundary determined by either a fixed
51-
* value ("httpclient_boundary_7k9p2m4x8n5j3q6t1r0vwyzabcdefghi") or a random UUID. If no
52-
* boundary is explicitly set via {@link #setBoundary(String)}, it defaults to the fixed
53-
* value unless {@link #withRandomBoundary()} is called to request a random UUID at build
54-
* time. Users can provide a custom boundary with {@link #setBoundary(String)}. A warning
55-
* is logged when no explicit boundary is set via {@link #setBoundary(String)}, encouraging
56-
* deliberate choice.
57-
* </p>
58-
* <p>
59-
* IMPORTANT: it is responsibility of the caller to validate / sanitize content of body
60-
* parts, for instance, to ensure they do not contain the boundary value that can prevent
61-
* the consumer of the entity from correctly parsing / processing the body parts.
50+
* This class constructs multipart entities with a boundary determined by either a random UUID
51+
* or an explicit boundary set via {@link #setBoundary(String)}.
6252
* </p>
6353
*
6454
* @since 5.0
@@ -74,7 +64,6 @@ public class MultipartEntityBuilder {
7464

7565
private static final String BOUNDARY_PREFIX = "httpclient_boundary_";
7666

77-
private boolean isRandomBoundaryRequested = false;
7867
/**
7968
* The logger for this class.
8069
*/
@@ -125,12 +114,14 @@ public MultipartEntityBuilder setStrictMode() {
125114
/**
126115
* Sets a custom boundary string for the multipart entity.
127116
* <p>
128-
* If {@code null} is provided, the builder reverts to its default boundary logic:
129-
* either using a boundary from the {@code contentType} if present, or falling back
130-
* to a fixed or random boundary (depending on {@link #withRandomBoundary()}).
117+
* If {@code null} is provided, the builder reverts to its default logic of using a random UUID.
118+
* </p>
119+
* <p>
120+
* IMPORTANT: when setting an explicit boundary, it is responsibility of the caller to validate / sanitize content
121+
* of body parts to ensure they do not contain the boundary value.
131122
* </p>
132123
*
133-
* @param boundary the boundary string, or {@code null} to use the default boundary logic
124+
* @param boundary the boundary string, or {@code null} to use a random UUID.
134125
* @return this builder instance
135126
*/
136127
public MultipartEntityBuilder setBoundary(final String boundary) {
@@ -234,14 +225,12 @@ public MultipartEntityBuilder addBinaryBody(final String name, final InputStream
234225
}
235226

236227
/**
237-
* Returns the fixed default boundary value.
238-
*/
239-
private String getFixedBoundary() {
240-
return BOUNDARY_PREFIX + "7k9p2m4x8n5j3q6t1r0vwyzabcdefghi";
241-
}
242-
243-
/**
244-
* Generates a random boundary using UUID.
228+
* Generates a random boundary using UUID. The UUID is a v4 random UUID generated from a cryptographically-secure
229+
* random source.
230+
* <p>
231+
* A cryptographically-secure random number source is used to avoid security issues similar to
232+
* CVE-2025-22150 (affecting the Node.JS ecosystem).
233+
* </p>
245234
*/
246235
private String getRandomBoundary() {
247236
return BOUNDARY_PREFIX + UUID.randomUUID();
@@ -274,29 +263,13 @@ public MultipartEntityBuilder addEpilogue(final String epilogue) {
274263
return this;
275264
}
276265

277-
/**
278-
* Configures the builder to request a random boundary generated by UUID.randomUUID()
279-
* at build time if no explicit boundary is set via {@link #setBoundary(String)}.
280-
*
281-
* @return this builder instance
282-
* @since 5.5
283-
*/
284-
public MultipartEntityBuilder withRandomBoundary() {
285-
this.isRandomBoundaryRequested = true;
286-
this.boundary = null;
287-
return this;
288-
}
289-
290266
MultipartFormEntity buildEntity() {
291267
String boundaryCopy = boundary;
292268
if (boundaryCopy == null && contentType != null) {
293269
boundaryCopy = contentType.getParameter("boundary");
294270
}
295271
if (boundaryCopy == null) {
296-
boundaryCopy = isRandomBoundaryRequested ? getRandomBoundary() : getFixedBoundary();
297-
if (LOG.isWarnEnabled()) {
298-
LOG.warn("No boundary explicitly set; using generated default: {}", boundaryCopy);
299-
}
272+
boundaryCopy = getRandomBoundary();
300273
}
301274
Charset charsetCopy = charset;
302275
if (charsetCopy == null && contentType != null) {

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

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,8 @@ void testMultipartWriteToRFC7578ModeWithFilenameStar() throws Exception {
313313
@Test
314314
void testRandomBoundary() {
315315
final MultipartFormEntity entity = MultipartEntityBuilder.create()
316-
.withRandomBoundary()
317316
.buildEntity();
318-
final NameValuePair boundaryParam = extractBoundary(entity.getContentType());
317+
final NameValuePair boundaryParam = extractBoundary(entity.getContentType(), "multipart/mixed");
319318
final String boundary = boundaryParam.getValue();
320319
Assertions.assertNotNull(boundary);
321320
Assertions.assertEquals(56, boundary.length());
@@ -324,21 +323,43 @@ void testRandomBoundary() {
324323
}
325324

326325
@Test
327-
void testExplicitBoundaryOverridesRandom() {
328-
final String customBoundary = "my_custom_boundary";
326+
void testRandomBoundaryWriteTo() throws Exception {
327+
final String helloWorld = "hello world";
328+
final List<NameValuePair> parameters = new ArrayList<>();
329+
parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_NAME, "test"));
330+
parameters.add(new BasicNameValuePair(MimeConsts.FIELD_PARAM_FILENAME, helloWorld));
329331
final MultipartFormEntity entity = MultipartEntityBuilder.create()
330-
.withRandomBoundary()
331-
.setBoundary(customBoundary)
332+
.setStrictMode()
333+
.addPart(new FormBodyPartBuilder()
334+
.setName("test")
335+
.setBody(new StringBody("hello world", ContentType.TEXT_PLAIN))
336+
.addField("Content-Disposition", "multipart/form-data", parameters)
337+
.build())
332338
.buildEntity();
333-
final NameValuePair boundaryParam = extractBoundary(entity.getContentType());
334-
Assertions.assertEquals(customBoundary, boundaryParam.getValue());
339+
340+
final NameValuePair boundaryParam = extractBoundary(entity.getContentType(), "multipart/form-data");
341+
final String boundary = boundaryParam.getValue();
342+
Assertions.assertNotNull(boundary);
343+
Assertions.assertEquals(56, boundary.length());
344+
Assertions.assertTrue(boundary.startsWith("httpclient_boundary_"));
345+
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}"));
346+
347+
final ByteArrayOutputStream out = new ByteArrayOutputStream();
348+
entity.writeTo(out);
349+
out.close();
350+
Assertions.assertEquals("--" + boundary + "\r\n" +
351+
"Content-Disposition: multipart/form-data; name=\"test\"; filename=\"hello world\"\r\n" +
352+
"Content-Type: text/plain; charset=UTF-8\r\n" +
353+
"\r\n" +
354+
helloWorld + "\r\n" +
355+
"--" + boundary + "--\r\n", out.toString(StandardCharsets.US_ASCII.name()));
335356
}
336357

337-
private NameValuePair extractBoundary(final String contentType) {
358+
private NameValuePair extractBoundary(final String contentType, final String expectedName) {
338359
final BasicHeaderValueParser parser = BasicHeaderValueParser.INSTANCE;
339360
final ParserCursor cursor = new ParserCursor(0, contentType.length());
340361
final HeaderElement elem = parser.parseHeaderElement(contentType, cursor);
341-
Assertions.assertEquals("multipart/mixed", elem.getName());
362+
Assertions.assertEquals(expectedName, elem.getName());
342363
return elem.getParameterByName("boundary");
343364
}
344365

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ void testImplicitContractorParams() {
7878
final String boundary = p1.getValue();
7979
Assertions.assertNotNull(boundary);
8080

81-
Assertions.assertEquals(52, boundary.length());
81+
// "httpclient_boundary_" + UUID (32 characters + 4 dashes) + 56 characters
82+
Assertions.assertEquals(56, boundary.length());
8283
final NameValuePair p2 = elem.getParameterByName("charset");
8384
Assertions.assertNull(p2);
8485
}

0 commit comments

Comments
 (0)