Code Review Request: 7152856: TEST_BUG: sun/net/www/protocol/jar/B4957695.java failing on Windows

[Chris Hegarty]

ship it!

-Chris.

On 17/04/12 19:27, Kurchi Hazra wrote:
> Updated webrev:
> http://cr.openjdk.java.net/~khazra/7152856/webrev.02/
>
> Alan: On second thoughts, both is and os were not required at all. I
> removed them.
>
> - Kurchi
>
>
> On 4/17/2012 8:47 AM, Kurchi Subhra Hazra wrote:
>> I think the HTTP spec needs an http server to handle LF gracefully,
>> although it expects a CRLF, and that is how this is working.
>> It is a small change, I will anyway correct it.
>>
>> - Kurchi
>>
>>
>> On 4/17/12 3:24 AM, Chris Hegarty wrote:
>>> On 16/04/12 22:18, Kurchi Hazra wrote:
>>>> Hi,
>>>>
>>>> Thanks for the reviews. Here is an updated webrev:
>>>> http://cr.openjdk.java.net/~khazra/7152856/webrev.01
>>>
>>> The updated webrev looks ok, but the "canned" HTTP response looks funny.
>>>
>>> Each HTTP header must be followed by a CRLF ( '\r\n' ), and the end
>>> of the headers ( just before the response body ) marked by CRLF CRLF.
>>> Is OutputStreamWriter somehow appending a new line?
>>>
>>> Sorry, I think at one time I pointed you to another test that may not
>>> have been sending a valid HTTP response.
>>>
>>> -Chris.
>>>
>>>>
>>>> Thanks,
>>>> Kurchi
>>>>
>>>>
>>>> On 4/15/2012 12:35 AM, Chris Hegarty wrote:
>>>>> On 14/04/12 16:53, Alan Bateman wrote:
>>>>>> On 13/04/2012 17:59, Kurchi Hazra wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> This test was failing on Windows since it was using the
>>>>>>> HttpServer in
>>>>>>> test/sun/net/www/httptest. The HttpServer implementation
>>>>>>> there is buggy and does not close the connection properly, resulting
>>>>>>> in the test hanging. We therefore write our own server, which sends
>>>>>>> back
>>>>>>> 10 bytes less than what the client expects, and see if the client
>>>>>>> raises an IOException.
>>>>>>>
>>>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7152856
>>>>>>> Webrev: http://cr.openjdk.java.net/~khazra/7152856/webrev.00
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Kurchi
>>>>>> Thanks for sorting out this test. A couple of comments:
>>>>>>
>>>>>> - I don't think the @run is right as samevm or agentvm is
>>>>>> specified to
>>>>>> jtreg rather than on specific tests (it is possible to add
>>>>>> /othervm to
>>>>>> force a test to run in its own VM).
>>>>>>
>>>>>> - "Server" might be better than XServer (as X server normally means a
>>>>>> X11 server).
>>>>>>
>>>>>> - XServer.srv should be final.
>>>>>>
>>>>>> - It looks like the server socket is closed when the test terminates.
>>>>>> Also to ensure that the accepted connection is closed I would suggest
>>>>>> that run be changed to try (Socket s = srv.accept()) { ... }.
>>>>>>
>>>>>> Otherwise I think it's okay.
>>>>>
>>>>> I agree with Alan's comments.
>>>>>
>>>>> Just to add, no @run tag is needed in this test. The default "@run
>>>>> main <name>" [1] should be fine, and allow the test be run in the mode
>>>>> specified by the caller. I think this is best where possible.
>>>>>
>>>>> "If no @run tags are present in a defining file, a default is assumed
>>>>> depending upon the file's filename extension. For a ".java" file,
>>>>> "@run main <name>" is assumed, where <name> is the name of the file
>>>>> without the ".java" suffix. For a ".sh" file, "@run shell <file>"
>>>>> is assumed. For an ".html" file, "@run applet <file>" is assumed."
>>>>>
>>>>> -Chris.
>>>>>
>>>>> [1] http://openjdk.java.net/jtreg/tag-spec.txt
>>>>>
>>>>>>
>>>>>> -Alan.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>