Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use random UUID for multipart boundary delimiter #916

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -31,7 +31,7 @@
* and <a href="http://tools.ietf.org/html/rfc1521#section-7.2.2">RFC 2046: Multipurpose Internet
* Mail Extensions: The Multipart/mixed (primary) subtype</a>.
*
* <p>By default the media type is {@code "multipart/related; boundary=__END_OF_PART__"}, but this
* <p>By default the media type is {@code "multipart/related; boundary=__END_OF_PART__<random UUID>__"}, but this
* may be customized by calling {@link #setMediaType(HttpMediaType)}, {@link #getMediaType()}, or
* {@link #setBoundary(String)}.
*
Expand All @@ -47,10 +47,14 @@ public class MultipartContent extends AbstractHttpContent {
private static final String TWO_DASHES = "--";

/** Parts of the HTTP multipart request. */
private ArrayList<Part> parts = new ArrayList<Part>();
private ArrayList<Part> parts = new ArrayList<>();

public MultipartContent() {
super(new HttpMediaType("multipart/related").setParameter("boundary", "__END_OF_PART__"));
this("__END_OF_PART__" + java.util.UUID.randomUUID().toString() + "__");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason not to import java.util.UUID? e.g a conflict with a similarly named class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

}

public MultipartContent(String boundary) {
super(new HttpMediaType("multipart/related").setParameter("boundary", boundary));
}

public void writeTo(OutputStream out) throws IOException {
Expand Down Expand Up @@ -152,7 +156,7 @@ public MultipartContent addPart(Part part) {
* changing the return type, but nothing else.
*/
public MultipartContent setParts(Collection<Part> parts) {
this.parts = new ArrayList<Part>(parts);
this.parts = new ArrayList<>(parts);
return this;
}

Expand All @@ -164,7 +168,7 @@ public MultipartContent setParts(Collection<Part> parts) {
* changing the return type, but nothing else.
*/
public MultipartContent setContentParts(Collection<? extends HttpContent> contentParts) {
this.parts = new ArrayList<Part>(contentParts.size());
this.parts = new ArrayList<>(contentParts.size());
for (HttpContent contentPart : contentParts) {
addPart(new Part(contentPart));
}
Expand Down
Expand Up @@ -26,55 +26,76 @@
*/
public class MultipartContentTest extends TestCase {

private static final String BOUNDARY = "__END_OF_PART__";
private static final String CRLF = "\r\n";
private static final String CONTENT_TYPE = Json.MEDIA_TYPE;
private static final String HEADERS =
"Content-Length: 3"
+ CRLF
+ "Content-Type: application/json; charset=UTF-8"
+ CRLF
+ "content-transfer-encoding: binary"
+ CRLF;
private static final String HEADERS = headers("application/json; charset=UTF-8", "foo");

private static String headers(String contentType, String value) {
return "Content-Length: " + value.length() + CRLF
+ "Content-Type: " + contentType + CRLF
+ "content-transfer-encoding: binary" + CRLF;
}

public void testRandomContent() throws Exception {
MultipartContent content = new MultipartContent();
String boundaryString = content.getBoundary();
assertNotNull(boundaryString);
assertTrue(boundaryString.startsWith(BOUNDARY));
assertTrue(boundaryString.endsWith("__"));
assertEquals("multipart/related; boundary=" + boundaryString, content.getType());

final String[][] VALUES = new String[][] {
{"Hello world", "text/plain"},
{"<xml>Hi</xml>", "application/xml"},
{"{x:1,y:2}", "application/json"}
};
StringBuilder expectedContentSB = new StringBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no abbreviations per google style, but simply "expectedContent" or even just expected is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expectedContent is used later. renamed to expectedStringBuilder

for (String[] valueTypePair: VALUES) {
String contentValue = valueTypePair[0];
String contentType = valueTypePair[1];
content.addPart(new MultipartContent.Part(ByteArrayContent.fromString(contentType, contentValue)));
expectedContentSB.append("--").append(boundaryString).append(CRLF)
.append(headers(contentType, contentValue)).append(CRLF)
.append(contentValue).append(CRLF);
}
expectedContentSB.append("--").append(boundaryString).append("--").append(CRLF);
// write to string
ByteArrayOutputStream out = new ByteArrayOutputStream();
content.writeTo(out);
String expectedContent = expectedContentSB.toString();
assertEquals(expectedContent, out.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out.toString needs to specify a locale

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in two places. Thanks for catching that.

assertEquals(StringUtils.getBytesUtf8(expectedContent).length, content.getLength());
}

public void testContent() throws Exception {
subtestContent("--__END_OF_PART__--" + CRLF, null);
subtestContent("--" + BOUNDARY + "--" + CRLF, null);
subtestContent(
"--__END_OF_PART__" + CRLF + HEADERS + CRLF + "foo" + CRLF + "--__END_OF_PART__--" + CRLF,
null,
"--" + BOUNDARY + CRLF
+ HEADERS + CRLF
+ "foo" + CRLF
+ "--" + BOUNDARY + "--" + CRLF,
null,
"foo");
subtestContent(
"--__END_OF_PART__"
+ CRLF
+ HEADERS
+ CRLF
+ "foo"
+ CRLF
+ "--__END_OF_PART__"
+ CRLF
+ HEADERS
+ CRLF
+ "bar"
+ CRLF
+ "--__END_OF_PART__--"
+ CRLF,
null,
"--" + BOUNDARY + CRLF
+ HEADERS + CRLF
+ "foo" + CRLF
+ "--" + BOUNDARY + CRLF
+ HEADERS + CRLF
+ "bar" + CRLF
+ "--" + BOUNDARY + "--" + CRLF,
null,
"foo",
"bar");
subtestContent(
"--myboundary"
+ CRLF
+ HEADERS
+ CRLF
+ "foo"
+ CRLF
+ "--myboundary"
+ CRLF
+ HEADERS
+ CRLF
+ "bar"
+ CRLF
+ "--myboundary--"
+ CRLF,
"--myboundary" + CRLF
+ HEADERS + CRLF
+ "foo" + CRLF
+ "--myboundary" + CRLF
+ HEADERS + CRLF
+ "bar" + CRLF
+ "--myboundary--" + CRLF,
"myboundary",
"foo",
"bar");
Expand All @@ -83,7 +104,7 @@ public void testContent() throws Exception {
private void subtestContent(String expectedContent, String boundaryString, String... contents)
throws Exception {
// multipart content
MultipartContent content = new MultipartContent();
MultipartContent content = new MultipartContent(boundaryString == null ? BOUNDARY : boundaryString);
for (String contentValue : contents) {
content.addPart(
new MultipartContent.Part(ByteArrayContent.fromString(CONTENT_TYPE, contentValue)));
Expand All @@ -98,7 +119,7 @@ private void subtestContent(String expectedContent, String boundaryString, Strin
assertEquals(StringUtils.getBytesUtf8(expectedContent).length, content.getLength());
assertEquals(
boundaryString == null
? "multipart/related; boundary=__END_OF_PART__"
? "multipart/related; boundary=" + BOUNDARY
: "multipart/related; boundary=" + boundaryString,
content.getType());
}
Expand Down