jersey
  1. jersey
  2. JERSEY-626

Some external multipart parser having trouble with the requests generated by Jersey Multipart

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.6
    • Component/s: None
    • Labels:
      None

      Description

      Jersey Multipart has a number of issues that affect generated multipart requests when they are sent to some servers, in particular when sending to Ruby on Rails applications where the parser seems to be quite brittle with regard to white spaces. For example, Jersey Multipart is placing an additional line return before the start of the first part in the generated request:

      POST /url HTTP/1.1
      User-Agent: Java/1.6.0_06
      Host: localhost
      Accept: */*
      Connection: keep-alive
      Content-Length: 609205
      Content-Type: multipart/form-data; boundary=----------------------------a4fc1bde1c45
      --line 1--
      --line 2--
      ------------------------------a4fc1bde1c45
      

      The same request in Curl:

      POST /url HTTP/1.1
      User-Agent: curl/7.18.2 (i386-redhat-linux-gnu) libcurl/7.18.2 NSS/3.12.2.0 zlib/1.2.3 libidn/0.6.14 libssh2/0.18
      Host: localhost
      Accept: */*
      Connection: keep-alive
      Content-Length: 609203
      Content-Type: multipart/form-data; boundary=----------------------------a4fc1bde1c45
       --line 1--
      ------------------------------a4fc1bde1c45
      

      As you can see when compared to Curl, Jersey Multipart is placing an additional line return before the start of the first part. In my experience this is breaking multipart parsing in Rails applications (and perhaps others?). The code in question is in the MultiPartWriter.writeTo() method, I fixed this issue by overriding this class and commenting out the leading line return:

      // Write the leading boundary string
      //writer.write("\r\n--");
      writer.write("--");
      writer.write(boundaryString);
      writer.write("\r\n");
      

      The second problem I experienced relates to the compression of white spaces in headers with parameters like Content-Disposition:

      Curl: Content-Type: multipart/form-data; boundary=----------------------------a4fc1bde1c45
      Jersey: Content-Type: multipart/form-data;boundary=----------------------------a4fc1bde1c45

      For some reason, Rails is sensitive to the removal of the spaces after the ;. While this may not be a requirement in HTTP, I suspect this convention is followed quite literally in some implementations. The problem lies in the MediaTypeProvider.toString() method:

          public String toString(MediaType header) {
              StringBuilder b = new StringBuilder();
              b.append(header.getType()).
                  append('/').
                  append(header.getSubtype());
              for (Map.Entry<String, String> e : header.getParameters().entrySet()) {
                  b.append("; "). // previously this was b.append(';').
                      append(e.getKey()).
                      append('=');
                  WriterUtil.appendQuotedIfWhiteSpaceOrQuote(b, e.getValue());
              }
              return b.toString();
          }
      

        Activity

        Hide
        mrduguo added a comment -

        The httpclient latest stable release 4.0.3 behavior same as curl: with only single line break before first part.

        The latest released version of jersey 1.5-ea09 also contain extra line break.

        I created a patch against trunk (rev 4479) to remove the line break for first part. There are spaces already for ContentDisposition in trunk code base.

        Thanks!

        Show
        mrduguo added a comment - The httpclient latest stable release 4.0.3 behavior same as curl: with only single line break before first part. The latest released version of jersey 1.5-ea09 also contain extra line break. I created a patch against trunk (rev 4479) to remove the line break for first part. There are spaces already for ContentDisposition in trunk code base. Thanks!
        Hide
        sandoz added a comment -

        The patch is wrong: "isFirstPart" is never set to false. It should be:

        Index: MultiPartWriter.java
        — MultiPartWriter.java Base (BASE)
        +++ MultiPartWriter.java Locally Modified (Based On LOCAL)
        @@ -137,10 +137,16 @@

        final String boundaryString = boundaryMediaType.getParameters().get("boundary");
        // Iterate through the body parts for this message
        + boolean firstPart = true;
        for (final BodyPart bodyPart : entity.getBodyParts()) {

        // Write the leading boundary string
        + if (firstPart)

        { + writer.write("--"); + firstPart = false; + }

        else

        { writer.write("\r\n--"); + }

        writer.write(boundaryString);
        writer.write("\r\n");

        Show
        sandoz added a comment - The patch is wrong: "isFirstPart" is never set to false. It should be: Index: MultiPartWriter.java — MultiPartWriter.java Base (BASE) +++ MultiPartWriter.java Locally Modified (Based On LOCAL) @@ -137,10 +137,16 @@ final String boundaryString = boundaryMediaType.getParameters().get("boundary"); // Iterate through the body parts for this message + boolean firstPart = true; for (final BodyPart bodyPart : entity.getBodyParts()) { // Write the leading boundary string + if (firstPart) { + writer.write("--"); + firstPart = false; + } else { writer.write("\r\n--"); + } writer.write(boundaryString); writer.write("\r\n");
        Hide
        sandoz added a comment -

        Attach reproducible test case provided by Murali Krishnan.

        Show
        sandoz added a comment - Attach reproducible test case provided by Murali Krishnan.
        Hide
        mrduguo added a comment -

        You are right! Sorry for didn't test it properly!

        Thanks!

        Show
        mrduguo added a comment - You are right! Sorry for didn't test it properly! Thanks!
        Hide
        Martin Matula added a comment -

        Fixed in rev. 4558.

        Show
        Martin Matula added a comment - Fixed in rev. 4558.

          People

          • Assignee:
            Martin Matula
            Reporter:
            j_col
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: