Hi Jim
Thanks for diving into this. Fell free to create a JIRA ticket with a patch.
Are the other tests that fail on OpenJDK on your system?
On Fri, Jun 10, 2011 at 9:14 PM, Jim Talbut <jtal...@spudsoft.co.uk> wrote:
> 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
>
>
--
Claus Ibsen
-----------------
FuseSource
Email: cib...@fusesource.com
Web: http://fusesource.com
Twitter: davsclaus, fusenews
Blog: http://davsclaus.blogspot.com/
Author of Camel in Action: http://www.manning.com/ibsen/
