FileUploadBase does not handle quoted boundary specification, which is required by RFC 1521

• Key: FILEUPLOAD-40
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Unassigned
• Reporter: Peter Chase
• Affects Version/s: 1.0 Final

Environment

Operating System: All
Platform: All

Description

In FileUploadBase.parseRequest(), the boundary string is computed by:

int boundaryIndex = contentType.indexOf("boundary=");
byte[] boundary = contentType.substring(boundaryIndex + 9).getBytes();

This is OK if the boundary string is unquoted. But RFC 1521 states clearly
that boundary strings may, and in some cases must, be quoted.

The consequence is that a request containing a quoted boundary string fails to
parse. The exact behaviour is that the parsing completes "successfully" but no
FileItems are found.

The code should look for a leading quote and, if found, move on by one
character before reading the boundary string. Perhaps it should check for the
trailing quote, too. Your test suites should be extended to test this case.

The examples in RFC 1521 show double quotes being used, and that is what
provoked the failure in the system I am trying to build. However, single
quotes may also be allowed (it probably says yes or no somewhere in the RFC).

If confirmed to be correct, I think that this is quite a serious fault,
requiring rapid attention. I have not provided a proposed patch, at this
stage, but would be willing to do so, if asked.

Comments

[Christoffer Hammarström]

I suggest using the ParameterParser checked in by Oleg Kalnichevski in COM-643
to parse the boundary-parameter of the content-type-header.

[Martin Cooper]

• • • COM-886 has been marked as a duplicate of this bug. ***

[Raphaël Piéroni]

here is a patch that worked !!!

String boundaryString;
byte[] boundary;
if (contentType.charAt(boundaryIndex + 9) == '"' &&
contentType.charAt(contentType.length() - 1) == '"')
{ boundaryString = contentType.substring( boundaryIndex + 10, contentType.length() - 1); } else { boundaryString = contentType.substring( boundaryIndex + 9); }
boundary = boundaryString.getBytes();

[Jan Vervecken]

We are using a User-Agent that sends request like:

-8<--------- begin —
POST /UploadTest/upload.do HTTP/1.0
Host: w2kp-jv:8888
User-Agent: IP*Works! V5 HTTP/S Component - by /n software - www.nsoftware.com
Content-Type: multipart/form-data; boundary="boundaryoazJlhY="
Content-Length: 230

--boundaryoazJlhY=
Content-Disposition: form-data; name="file"; filename="C:\myUploadTest.txt"
Content-Type: text/plain

a first line
a second line
-boundaryoazJlhY=-

-8<--------- end —

Note the quotes on the value for the boundary in the Content-Type field.

I have made these code changes, they work for us:

-8<--------- begin —
Index: FileUploadBase.java
===================================================================
RCS file: /home/cvspublic/jakarta-
commons/fileupload/src/java/org/apache/commons/fileupload/FileUploadBase.java,v
retrieving revision 1.4
diff -u -r1.4 FileUploadBase.java
— FileUploadBase.java 9 Oct 2003 21:15:47 -0000 1.4
+++ FileUploadBase.java 5 Nov 2003 22:02:38 -0000
@@ -328,8 +328,17 @@
"the request was rejected because "
+ "no multipart boundary was found");
}

• byte[] boundary = contentType.substring(
• boundaryIndex + 9).getBytes();
  + String boundaryBase = contentType.substring(boundaryIndex + 9);
  + byte[] boundary = null;
  + if (boundaryBase.startsWith("\"") && boundaryBase.endsWith("\""))
  + { + // removing quotes + boundary = boundaryBase.substring(1, (boundaryBase.length() - 1)).getBytes(); + }
  + else
  + { + boundary = boundaryBase.getBytes(); + }

InputStream input = req.getInputStream();
-8<--------- end —

I'm not an expert on the related RFCs, but check the "WARNING TO IMPLEMENTORS"
on page 30 of http://www.ietf.org/rfc/rfc1521.txt

I hope this makes sense.

[Jan Vervecken]

Created an attachment (id=8950)
[PATCH] removing the boundary quotes

[Christoffer Hammarström]

Jan: Note that String.getBytes() uses the system default encoding, which is
hardly correct outside western locales. Should probably use "US-ASCII" or
"ISO-8859-1".
But, as i said earlier, it would probably be better to use Oleg's
ParameterParser (COM-643 / attachment 8205) consistently in commons fileupload.

[Martin Cooper]

Fixed in the 10/11/04 nightly build.

I followed Christoffer's suggestion and used ParameterParser.

[Martin Cooper]

• • • COM-1698 has been marked as a duplicate of this bug. ***