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 Sun, 12 Jun 2011 14:15:46 GMT
Claus,

It's CAMEL-4094.
Just started a full build again, so we'll see where that breaks :)

Jim

On 11/06/2011 10:57, Claus Ibsen wrote:
> 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<jtalbut@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
>>
>
>


Mime
View raw message