HttpPostMultipartRequestDecoder should decode header field parameters
Motivation: I am receiving a multipart/form_data upload from a Mailgun webhook. This webhook used to send parts like this: --74e78d11b0214bdcbc2f86491eeb4902 Content-Disposition: form-data; name="attachment-2"; filename="attached_�айл.txt" Content-Type: text/plain Content-Length: 32 This is the content of the file --74e78d11b0214bdcbc2f86491eeb4902-- but now it posts parts like this: --74e78d11b0214bdcbc2f86491eeb4902 Content-Disposition: form-data; name="attachment-2"; filename*=utf-8''attached_%D1%84%D0%B0%D0%B9%D0%BB.txt This is the content of the file --74e78d11b0214bdcbc2f86491eeb4902-- This new format uses field parameter encoding described in RFC 5987. More about this encoding can be found here. Netty does not parse this format. The result is the filename is not decoded and the part is not parsed into a FileUpload. Modification: Added failing test in HttpPostRequestDecoderTest.java and updated HttpPostMultipartRequestDecoder.java Refactored to please Netkins Result: Fixes: HttpPostMultipartRequestDecoder identifies the RFC 5987 format and parses it. Previous functionality is retained.
This commit is contained in:
Dmitry Minkovsky
Norman Maurer
parent
e62e6df4ac
commit
8aeba78ecc
|
|
@ -22,6 +22,7 @@ import io.netty.handler.codec.http.HttpHeaderNames;
|
|||
import io.netty.handler.codec.http.HttpHeaderValues;
|
||||
import io.netty.handler.codec.http.HttpRequest;
|
||||
import io.netty.handler.codec.http.LastHttpContent;
|
||||
import io.netty.handler.codec.http.QueryStringDecoder;
|
||||
import io.netty.handler.codec.http.multipart.HttpPostBodyUtil.SeekAheadOptimize;
|
||||
import io.netty.handler.codec.http.multipart.HttpPostBodyUtil.TransferEncodingMechanism;
|
||||
import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.EndOfDataDecoderException;
|
||||
|
|
@ -691,18 +692,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
|||
String[] values = contents[i].split("=", 2);
|
||||
Attribute attribute;
|
||||
try {
|
||||
String name = cleanString(values[0]);
|
||||
String value = values[1];
|
||||
|
||||
// See http://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html
|
||||
if (HttpHeaderValues.FILENAME.contentEquals(name)) {
|
||||
// filename value is quoted string so strip them
|
||||
value = value.substring(1, value.length() - 1);
|
||||
} else {
|
||||
// otherwise we need to clean the value
|
||||
value = cleanString(value);
|
||||
}
|
||||
attribute = factory.createAttribute(request, name, value);
|
||||
attribute = getContentDispositionAttribute(values);
|
||||
} catch (NullPointerException e) {
|
||||
throw new ErrorDataDecoderException(e);
|
||||
} catch (IllegalArgumentException e) {
|
||||
|
|
@ -805,6 +795,38 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
|||
}
|
||||
}
|
||||
|
||||
private static final String FILENAME_ENCODED = HttpHeaderValues.FILENAME.toString() + '*';
|
||||
|
||||
private Attribute getContentDispositionAttribute(String... values) {
|
||||
String name = cleanString(values[0]);
|
||||
String value = values[1];
|
||||
|
||||
// Filename can be token, quoted or encoded. See https://tools.ietf.org/html/rfc5987
|
||||
if (HttpHeaderValues.FILENAME.contentEquals(name)) {
|
||||
// Value is quoted or token. Strip if quoted:
|
||||
int last = value.length() - 1;
|
||||
if (last > 0 &&
|
||||
value.charAt(0) == HttpConstants.DOUBLE_QUOTE &&
|
||||
value.charAt(last) == HttpConstants.DOUBLE_QUOTE) {
|
||||
value = value.substring(1, last);
|
||||
}
|
||||
} else if (FILENAME_ENCODED.equals(name)) {
|
||||
try {
|
||||
name = HttpHeaderValues.FILENAME.toString();
|
||||
String[] split = value.split("'", 3);
|
||||
value = QueryStringDecoder.decodeComponent(split[2], Charset.forName(split[0]));
|
||||
} catch (ArrayIndexOutOfBoundsException e) {
|
||||
throw new ErrorDataDecoderException(e);
|
||||
} catch (UnsupportedCharsetException e) {
|
||||
throw new ErrorDataDecoderException(e);
|
||||
}
|
||||
} else {
|
||||
// otherwise we need to clean the value
|
||||
value = cleanString(value);
|
||||
}
|
||||
return factory.createAttribute(request, name, value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the FileUpload (new one or current one)
|
||||
*
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import io.netty.handler.codec.http.LastHttpContent;
|
|||
import io.netty.util.CharsetUtil;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.net.URLEncoder;
|
||||
import java.nio.charset.UnsupportedCharsetException;
|
||||
import java.util.Arrays;
|
||||
|
||||
|
|
@ -491,4 +492,138 @@ public class HttpPostRequestDecoderTest {
|
|||
content.release();
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/netty/netty/pull/7265
|
||||
@Test
|
||||
public void testDecodeContentDispositionFieldParameters() throws Exception {
|
||||
|
||||
final String boundary = "74e78d11b0214bdcbc2f86491eeb4902";
|
||||
|
||||
String encoding = "utf-8";
|
||||
String filename = "attached_файл.txt";
|
||||
String filenameEncoded = URLEncoder.encode(filename, encoding);
|
||||
|
||||
final String body = "--" + boundary + "\r\n" +
|
||||
"Content-Disposition: form-data; name=\"file\"; filename*=" + encoding + "''" + filenameEncoded + "\r\n" +
|
||||
"\r\n" +
|
||||
"foo\r\n" +
|
||||
"\r\n" +
|
||||
"--" + boundary + "--";
|
||||
|
||||
final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
|
||||
HttpMethod.POST,
|
||||
"http://localhost",
|
||||
Unpooled.wrappedBuffer(body.getBytes()));
|
||||
|
||||
req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
|
||||
final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req);
|
||||
assertFalse(decoder.getBodyHttpDatas().isEmpty());
|
||||
InterfaceHttpData part1 = decoder.getBodyHttpDatas().get(0);
|
||||
assertTrue("the item should be a FileUpload", part1 instanceof FileUpload);
|
||||
FileUpload fileUpload = (FileUpload) part1;
|
||||
assertEquals("the filename should be decoded", filename, fileUpload.getFilename());
|
||||
decoder.destroy();
|
||||
req.release();
|
||||
}
|
||||
|
||||
// https://github.com/netty/netty/pull/7265
|
||||
@Test
|
||||
public void testDecodeWithLanguageContentDispositionFieldParameters() throws Exception {
|
||||
|
||||
final String boundary = "74e78d11b0214bdcbc2f86491eeb4902";
|
||||
|
||||
String encoding = "utf-8";
|
||||
String filename = "attached_файл.txt";
|
||||
String language = "anything";
|
||||
String filenameEncoded = URLEncoder.encode(filename, encoding);
|
||||
|
||||
final String body = "--" + boundary + "\r\n" +
|
||||
"Content-Disposition: form-data; name=\"file\"; filename*=" +
|
||||
encoding + "'" + language + "'" + filenameEncoded + "\r\n" +
|
||||
"\r\n" +
|
||||
"foo\r\n" +
|
||||
"\r\n" +
|
||||
"--" + boundary + "--";
|
||||
|
||||
final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
|
||||
HttpMethod.POST,
|
||||
"http://localhost",
|
||||
Unpooled.wrappedBuffer(body.getBytes()));
|
||||
|
||||
req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
|
||||
final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req);
|
||||
assertFalse(decoder.getBodyHttpDatas().isEmpty());
|
||||
InterfaceHttpData part1 = decoder.getBodyHttpDatas().get(0);
|
||||
assertTrue("the item should be a FileUpload", part1 instanceof FileUpload);
|
||||
FileUpload fileUpload = (FileUpload) part1;
|
||||
assertEquals("the filename should be decoded", filename, fileUpload.getFilename());
|
||||
decoder.destroy();
|
||||
req.release();
|
||||
}
|
||||
|
||||
// https://github.com/netty/netty/pull/7265
|
||||
@Test
|
||||
public void testDecodeMalformedNotEncodedContentDispositionFieldParameters() throws Exception {
|
||||
|
||||
final String boundary = "74e78d11b0214bdcbc2f86491eeb4902";
|
||||
|
||||
final String body = "--" + boundary + "\r\n" +
|
||||
"Content-Disposition: form-data; name=\"file\"; filename*=not-encoded\r\n" +
|
||||
"\r\n" +
|
||||
"foo\r\n" +
|
||||
"\r\n" +
|
||||
"--" + boundary + "--";
|
||||
|
||||
final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
|
||||
HttpMethod.POST,
|
||||
"http://localhost",
|
||||
Unpooled.wrappedBuffer(body.getBytes()));
|
||||
|
||||
req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||
|
||||
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
|
||||
|
||||
try {
|
||||
new HttpPostRequestDecoder(inMemoryFactory, req);
|
||||
fail("Was expecting an ErrorDataDecoderException");
|
||||
} catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
|
||||
assertTrue(e.getCause() instanceof ArrayIndexOutOfBoundsException);
|
||||
} finally {
|
||||
req.release();
|
||||
}
|
||||
}
|
||||
|
||||
// https://github.com/netty/netty/pull/7265
|
||||
@Test
|
||||
public void testDecodeMalformedBadCharsetContentDispositionFieldParameters() throws Exception {
|
||||
|
||||
final String boundary = "74e78d11b0214bdcbc2f86491eeb4902";
|
||||
|
||||
final String body = "--" + boundary + "\r\n" +
|
||||
"Content-Disposition: form-data; name=\"file\"; filename*=not-a-charset''filename\r\n" +
|
||||
"\r\n" +
|
||||
"foo\r\n" +
|
||||
"\r\n" +
|
||||
"--" + boundary + "--";
|
||||
|
||||
final DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1,
|
||||
HttpMethod.POST,
|
||||
"http://localhost",
|
||||
Unpooled.wrappedBuffer(body.getBytes()));
|
||||
|
||||
req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||
|
||||
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
|
||||
|
||||
try {
|
||||
new HttpPostRequestDecoder(inMemoryFactory, req);
|
||||
fail("Was expecting an ErrorDataDecoderException");
|
||||
} catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
|
||||
assertTrue(e.getCause() instanceof UnsupportedCharsetException);
|
||||
} finally {
|
||||
req.release();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user