camel-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Talbut <jtal...@spudsoft.co.uk>
Subject Re: Build break in camel-jetty
Date Fri, 10 Jun 2011 19:14:04 GMT
I think both the test in camel-jetty and the code in camel-http are wrong.

  In camel-http (DefaultHttpBinding) we have:
     protected void populateAttachments(HttpServletRequest request, 
HttpMessage message) {
         // check if there is multipart files, if so will put it into 
DataHandler
         Enumeration names = request.getAttributeNames();
         while (names.hasMoreElements()) {
             String name = (String) names.nextElement();
             Object object = request.getAttribute(name);
             LOG.trace("HTTP attachment {} = {}", name, object);
             if (object instanceof File) {
                 String fileName = request.getParameter(name);
                 message.addAttachment(fileName, new DataHandler(new 
CamelFileDataSource((File)object, fileName)));
             }
         }
     }

That creates the DataHandler for the uploaded file without specifying 
the content-type.
So different implementations of DataHandler may well have different 
defaults (hence the difference between Sun and OpenJDK).
This is wrong, but I can't see how to get the correct content-type for 
the uploaded file in order to add it to the constructor args.

The test is explicitly testing that the DataHandler has the same 
defaults as the Sun JDK, which is not helpful.
It ought to test that the content-type is application/octet-stream, but 
that would require changing camel-http.

There are two options:
1. Fix camel-http so it does set the content-type correctly (though I 
couldn't work out where to get it from).
     We'd then have to fix the test to check for the correct content-type.
2. Remove the assertion from the test.

Jim


On 10/06/2011 07:20, Jim Talbut wrote:
> On 09/06/2011 19:54, Jim Talbut wrote:
>> On 09/06/2011 15:32, James Talbut wrote:
>>> On Thu, Jun 09, 2011 at 10:13:00PM +0800, Willem Jiang wrote:
>>>> Maybe it relates to the openjdk.
>>>> Can you try the latest SUN/Oracle JDK 1.6 to run the test ?
>>> That's possible, I should be able to try later today.
>>>
>>> I've also tried on Windows 7 and that works (breaks in camel-ftp, 
>>> but that's another matter).
>>>
>>> Jim
>> That was it, how interesting.
>>
>> I downloaded the Sun JDK, set JAVA_HOME to point to it and ran maven 
>> and it went correctly.
>>
>> Should Camel support alternative JVMs?
>>
>> Jim
> Curiouser and curiouser...
>
> I'm starting to think that it's actually the OpenJDK that's right and 
> the test is wrong.
> The test has:
>         Part[] parts = {
>             new StringPart("comment", "A binary file of some kind"),
>             new FilePart(file.getName(), file)
>         };
> There is nothing there to specify the content-type and there seems to 
> be an assumption that it's binary, not text (though it is actually text).
>
> The source for FilePart has:
> public static final String DEFAULT_CONTENT_TYPE = 
> "application/octet-stream";
> and that is used unless the content type is specified in the 
> constructor (which it isn't).
>
> So the string part should have a content type of "text/plain" and the 
> file part should have a content type of "application/octet-stream".
>
> The test has:
>                         DataHandler data = 
> in.getAttachment("NOTICE.txt");
>                         assertNotNull("Should get the DataHandle 
> NOTICE.txt", data);
>                         assertEquals("Get a wrong content type", 
> "text/plain", data.getContentType());
> Verifying that the file part is "text/plain".
>
> The on-the-wire data with OpenJDK is:
>
>    --lDIZtpp0tBIGfhQWzHIZ5kV7QpREJSj-Cmp3Uf8C
>    Content-Disposition: form-data; name="comment"
>    Content-Type: text/plain; charset=US-ASCII
>    Content-Transfer-Encoding: 8bit
>
>    A binary file of some kind
>    --lDIZtpp0tBIGfhQWzHIZ5kV7QpREJSj-Cmp3Uf8C
>    Content-Disposition: form-data; name="NOTICE.txt"; 
> filename="NOTICE.txt"
>    Content-Type: application/octet-stream; charset=ISO-8859-1
>    Content-Transfer-Encoding: binary
>
>          
> =========================================================================
>        ==  NOTICE file corresponding to the section 4 d
>    of                    ==
>        ==  the Apache License, Version
>    2.0,                                   ==
>        ==  in this case for the Apache Camel
>    distribution.                    ==
>          
> =========================================================================
>
>        This product includes software developed by
>        The Apache Software Foundation (http://www.apache.org/).
>
>        Please read the different LICENSE files present in the licenses
>    directory of
>        this distribution.
>
>    --lDIZtpp0tBIGfhQWzHIZ5kV7QpREJSj-Cmp3Uf8C--
>
> The on-the-wire data with Sun JDK is:
>
>    --qhLzs_ECq12ft9Tp3B1ayD-jjjzshV
>    Content-Disposition: form-data; name="comment"
>    Content-Type: text/plain; charset=US-ASCII
>    Content-Transfer-Encoding: 8bit
>
>    A binary file of some kind
>    --qhLzs_ECq12ft9Tp3B1ayD-jjjzshV
>    Content-Disposition: form-data; name="NOTICE.txt"; 
> filename="NOTICE.txt"
>    Content-Type: application/octet-stream; charset=ISO-8859-1
>    Content-Transfer-Encoding: binary
>
>          
> =========================================================================
>        ==  NOTICE file corresponding to the section 4 d
>    of                    ==
>        ==  the Apache License, Version
>    2.0,                                   ==
>        ==  in this case for the Apache Camel
>    distribution.                    ==
>          
> =========================================================================
>
>        This product includes software developed by
>        The Apache Software Foundation (http://www.apache.org/).
>
>        Please read the different LICENSE files present in the licenses
>    directory of
>        this distribution.
>
>    --qhLzs_ECq12ft9Tp3B1ayD-jjjzshV--
>
> Note that both have the correct content-type 
> (application/octet-stream) for the file.
>
> So I think there is a bug in camel-jetty when used with the Sun JDK.
> Still looking, but please stop me if you think I've gone wrong so far!
>
> Jim


Mime
View raw message