Skip to content

Commit

Permalink
HTTPCLIENT-2325 Avoid adding "; charset=" for multipart/form-data req…
Browse files Browse the repository at this point in the history
…uests

Previusly, "charset" parameter was added to the Content-Type header, however adding "charset=..."
is not specified in RFC 7578, and it causes issues with (flawed?) HTTP servers.

The change does not modify ContentType.MULTIPART_FORM_DATA, and it might have backward compatibility
side-effects.

See
* apache/jmeter#6250
* owasp-modsecurity/ModSecurity@6e56950
* https://bz.apache.org/bugzilla/show_bug.cgi?id=61384
* akka/akka-http#338
  • Loading branch information
vlsi committed Mar 18, 2024
1 parent ccff406 commit 76e89c9
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -253,12 +253,13 @@ MultipartFormEntity buildEntity() {
if (charsetCopy == null && contentType != null) {
charsetCopy = contentType.getCharset();
}
final List<NameValuePair> paramsList = new ArrayList<>(2);
paramsList.add(new BasicNameValuePair("boundary", boundaryCopy));
if (charsetCopy != null) {
paramsList.add(new BasicNameValuePair("charset", charsetCopy.name()));
}
final NameValuePair[] params = paramsList.toArray(EMPTY_NAME_VALUE_ARRAY);
// Previusly, "charset" parameter was added to the Content-Type header, however adding "charset=..."
// is not specified in RFC 7578, and it causes issues with (flawed?) HTTP servers.
// For instance:
// https://github.com/owasp-modsecurity/ModSecurity/commit/6e56950cdf258c9b39f12cf6eb014cb59797cfd3
// https://github.com/akka/akka-http/issues/338
// https://bz.apache.org/bugzilla/show_bug.cgi?id=61384
final NameValuePair[] params = new NameValuePair[]{new BasicNameValuePair("boundary", boundaryCopy)};

final ContentType contentTypeCopy;
if (contentType != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void testMultipartCustomContentType() throws Exception {
.setLaxMode()
.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("application/xml; boundary=blah-blah; charset=UTF-8", entity.getContentType());
Assertions.assertEquals("application/xml; charset=UTF-8; boundary=blah-blah", entity.getContentType());
}

@Test
Expand All @@ -99,11 +99,36 @@ public void testMultipartContentTypeParameter() throws Exception {
new BasicNameValuePair("charset", "ascii")))
.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("multipart/form-data; boundary=yada-yada; charset=US-ASCII", entity.getContentType());
Assertions.assertEquals("multipart/form-data; boundary=yada-yada; charset=ascii", entity.getContentType());
Assertions.assertEquals("yada-yada", entity.getMultipart().boundary);
Assertions.assertEquals(StandardCharsets.US_ASCII, entity.getMultipart().charset);
}

@Test
public void testMultipartDefaultContentTypeOmitsCharset() throws Exception {
final MultipartFormEntity entity = MultipartEntityBuilder.create()
.setCharset(StandardCharsets.UTF_8)
.setBoundary("yada-yada")
.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("multipart/mixed; boundary=yada-yada", entity.getContentType());
Assertions.assertEquals("yada-yada", entity.getMultipart().boundary);
}

@Test
public void testMultipartFormDataContentTypeOmitsCharset() throws Exception {
// Note: org.apache.hc.core5.http.ContentType.MULTIPART_FORM_DATA uses StandardCharsets.ISO_8859_1,
// so we create a custom ContentType here
final MultipartFormEntity entity = MultipartEntityBuilder.create()
.setContentType(ContentType.create("multipart/form-data"))
.setCharset(StandardCharsets.UTF_8)
.setBoundary("yada-yada")
.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("multipart/form-data; boundary=yada-yada", entity.getContentType());
Assertions.assertEquals("yada-yada", entity.getMultipart().boundary);
}

@Test
public void testMultipartCustomContentTypeParameterOverrides() throws Exception {
final MultipartFormEntity entity = MultipartEntityBuilder.create()
Expand All @@ -116,7 +141,7 @@ public void testMultipartCustomContentTypeParameterOverrides() throws Exception
.setLaxMode()
.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("multipart/form-data; boundary=blah-blah; charset=UTF-8; my=stuff",
Assertions.assertEquals("multipart/form-data; boundary=blah-blah; charset=ascii; my=stuff",
entity.getContentType());
}

Expand All @@ -130,7 +155,7 @@ public void testMultipartCustomContentTypeUsingAddParameter() {
eb.buildEntity();
final MultipartFormEntity entity = eb.buildEntity();
Assertions.assertNotNull(entity);
Assertions.assertEquals("multipart/related; boundary=yada-yada; charset=US-ASCII; my=stuff",
Assertions.assertEquals("multipart/related; boundary=yada-yada; charset=ascii; my=stuff",
entity.getContentType());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ public void testExplictContractorParams() throws Exception {
Assertions.assertNotNull(p1);
Assertions.assertEquals("whatever", p1.getValue());
final NameValuePair p2 = elem.getParameterByName("charset");
Assertions.assertNotNull(p2);
Assertions.assertEquals("UTF-8", p2.getValue());
Assertions.assertNull(p2,
"RFC7578 does not mention charset parameter for Content-Type, " +
"so no charset should be present for MultipartEntity.getContentType()");
}

@Test
Expand Down

0 comments on commit 76e89c9

Please sign in to comment.