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.

Sign up for GitHub

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

Jump to bottom

HTTP multipart request with filename containing ";" causes exception #3326

Closed
dittos opened this issue Jan 13, 2015 · 2 comments
Closed

HTTP multipart request with filename containing ";" causes exception #3326

dittos opened this issue Jan 13, 2015 · 2 comments
Assignees
@normanmaurer
Labels
defect
Milestone
3.10.1.Final

Comments

@dittos
Copy link

dittos commented Jan 13, 2015

Netty version: 3.10.0.Final

When I send HTTP request something like:

POST / HTTP/1.1
Content-Type: multipart/form-data; boundary=dLV9Wyq26L_-JQxk6ferf-RT153LhOO
--dLV9Wyq26L_-JQxk6ferf-RT153LhOO
Content-Disposition: form-data; name="file"; filename="tmp;0.txt"
Content-Type: image/gif

asdf
--dLV9Wyq26L_-JQxk6ferf-RT153LhOO--

It causes the following stack trace.

java.lang.ArrayIndexOutOfBoundsException: 1
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.findMultipartDisposition(HttpPostMultipartRequestDecoder.java:540)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.decodeMultipart(HttpPostMultipartRequestDecoder.java:345)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.findMultipartDelimiter(HttpPostMultipartRequestDecoder.java:482)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.decodeMultipart(HttpPostMultipartRequestDecoder.java:332)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.parseBodyMultipart(HttpPostMultipartRequestDecoder.java:294)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.parseBody(HttpPostMultipartRequestDecoder.java:265)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder.<init>(HttpPostMultipartRequestDecoder.java:167)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostRequestDecoder.<init>(HttpPostRequestDecoder.java:80)
    at org.jboss.netty.handler.codec.http.multipart.HttpPostRequestDecoder.<init>(HttpPostRequestDecoder.java:56)
    ...

Test case (add to HttpPostRequestDecoderTest):

final String boundary = "dLV9Wyq26L_-JQxk6ferf-RT153LhOO";

final DefaultHttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST,
        "http://localhost");

req.headers().add(HttpHeaders.Names.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);

// Force to use memory-based data.
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);

final String data = "asdf";
final String filename = "tmp;0.txt";

final String body =
        "--" + boundary + "\r\n" +
                "Content-Disposition: form-data; name=\"file\"; filename=\"" + filename + "\"\r\n" +
                "Content-Type: image/gif\r\n" +
                "\r\n" +
                data + "\r\n" +
                "--" + boundary + "--\r\n";

req.setContent(ChannelBuffers.wrappedBuffer(body.getBytes(CharsetUtil.UTF_8.name())));
// Create decoder instance to test.
final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req);
assertFalse(decoder.getBodyHttpDatas().isEmpty());
assertEquals(filename, ((MemoryFileUpload) decoder.getBodyHttpDatas().get(0)).getFilename());

Although I only tested 3.x release, but it seems that also 4.x/5.0 have the same bug.
@fredericBregier
Copy link
Member

@dittos I confirm.
The issue is in splitMultipartHeader where, once Content-Disposition is found, we split the data according to ';'. In this case, this is wrong.
Nothing prevents a filename to include a ";" (even if ';' is really not compatible with most of OS like Linux).
I do not have time right now to make a fix, but I feel like the splitMultipartHeader should be changed such that we split the additionnal arguments using ';' but including a check on ' " ' pairs in order to not check within this pair.
Something like:

  • find first "
  • find ; (possible multiple) between current position and first " and split (keeping only right side), updating current position
  • loop on previous step as long as we found ;
  • find next ", set new position to after it
  • loop on first item until position is not the end

@fredericBregier
Copy link
Member

@dittos could you propose a fix and a test case associated as you did already ?

dittos added a commit to dittos/netty that referenced this issue Jan 14, 2015
@dittos
Accept ';' in the filename of HTTP Content-Disposition header
13d5393 
Motivation:

HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException when
trying to decode Content-Disposition header with filename containing ';'.
See issue netty#3326.

Modifications:

Added splitMultipartHeaderValues method which cares about quotes, and use it in
splitMultipartHeader method, instead of StringUtils.split.

Result:

Filenames can contain semicolons.
@dittos dittos mentioned this issue Jan 14, 2015
Closed
normanmaurer pushed a commit that referenced this issue Jan 15, 2015
@dittos @normanmaurer
Accept ';' in the filename of HTTP Content-Disposition header
add0a0c 
Motivation:

HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException when
trying to decode Content-Disposition header with filename containing ';'.
See issue #3326.

Modifications:

Added splitMultipartHeaderValues method which cares about quotes, and use it in
splitMultipartHeader method, instead of StringUtils.split.

Result:

Filenames can contain semicolons.
fredericBregier added a commit to fredericBregier/netty that referenced this issue Jan 15, 2015
@fredericBregier
Accept ';' '\\"' in the filename of HTTP Content-Disposition header
a497efc 
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue netty#3326 and netty#3327.

Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.

Result:
Filenames can contain semicolons and protected \\".
@fredericBregier fredericBregier mentioned this issue Jan 15, 2015
Closed
fredericBregier added a commit that referenced this issue Jan 16, 2015
@fredericBregier @normanmaurer
Accept ';' '\\"' in the filename of HTTP Content-Disposition header
6ecc67f 
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue #3326 and #3327.

Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.

Result:
Filenames can contain semicolons and protected \\".
fredericBregier added a commit that referenced this issue Jan 16, 2015
@fredericBregier @normanmaurer
Accept ';' '\\"' in the filename of HTTP Content-Disposition header
7f907e8 
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue #3326 and #3327.

Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.

Result:
Filenames can contain semicolons and protected \\".
fredericBregier added a commit that referenced this issue Jan 16, 2015
@fredericBregier @normanmaurer
Accept ';' '\\"' in the filename of HTTP Content-Disposition header
cbc54f9 
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue #3326 and #3327.

Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.

Result:
Filenames can contain semicolons and protected \\".
@normanmaurer normanmaurer self-assigned this Jan 16, 2015
@normanmaurer normanmaurer added this to the 3.10.1.Final milestone Jan 16, 2015
pulllock pushed a commit to pulllock/netty that referenced this issue Oct 19, 2023
@fredericBregier @normanmaurer
Accept ';' '\\"' in the filename of HTTP Content-Disposition header
7105190 
Motivation:
HttpPostMultipartRequestDecoder threw an ArrayIndexOutOfBoundsException
when trying to decode Content-Disposition header with filename
containing ';' or protected \\".
See issue netty#3326 and netty#3327.

Modifications:
Added splitMultipartHeaderValues method which cares about quotes, and
use it in splitMultipartHeader method, instead of StringUtils.split.

Result:
Filenames can contain semicolons and protected \\".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@normanmaurer normanmaurer

Labels
defect
Projects
None yet
Milestone
3.10.1.Final
Development

No branches or pull requests
3 participants
@dittos @normanmaurer @fredericBregier