Hello Daniel, On 12/06/19 3:42 PM, Daniel Fuchs wrote: > Hi Jaikiran, > > I have eyeballed your patch and on the surface I'd say it looks > good to me. I'll need to have a deeper look at the full code > after importing it but I'm not expecting any surprise. > > I can work as your sponsor for this. Thank you :) > Thanks for adding the new 3 digits check and finding > the appropriate test for this :-). > > Please give me some time to import your patch and send it > through our test system - and try to shake it a bit :-) Please take your time. > > If I'm not mistaken you're a JDK author [1] right? Yes, I am. > If so you should have your own web page [2] available > to publish (you can use scp) webrevs [3] should you wish > to do so (webrevs are usually more convenient to reviewers > as they contain the full context) I wasn't aware that the author status allows me to create webrev hosted at cr.openjdk.java.net. Thanks for pointing me to that. I read up a bit and have tested my access to that server and it's working fine. > > We're not quite there yet as I still need to test > your patch first - but since you are an author you > should also be prepared to generate a changeset > with a properly formatted comment [4] that can > be directly hg imported and then pushed to the > jdk repo.
I decided to give it a try and host it on cr.openjdk.java.net. I have now published the webrev for this patch, here http://cr.openjdk.java.net/~jpai/8217705/00/webrev/ This is my first usage of this tool, so if there's something that I missed or got wrong, please do let me know. And of course, if the webrev needs to be regenerated after you are done testing the patch, I can do that too. Thank you for your help so far. -Jaikiran > > best regards, > > -- daniel > > [1] http://openjdk.java.net/census#jpai > [2] http://cr.openjdk.java.net/~jpai/ > [3] http://openjdk.java.net/projects/code-tools/ > [4] http://openjdk.java.net/guide/producingChangeset.html > > On 12/06/2019 02:46, Jaikiran Pai wrote: >> Hello, >> >> Attached is a patch for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8217705. >> >> In addition to catching the NumberFormatException that can arise while >> parsing (an invalid) status code in the status line, this change also >> checks that the status code is indeed a 3-digit integer, as required by >> the RFC-2616, section 6.1.1 [1]. In either of these cases, where the >> status code is incorrect, this change now throws a >> java.net.ProtocolException similar to other cases where it's thrown for >> issues encountered during parsing of the status line. >> >> The patch also contains an update to an existing test case to include >> testing of these invalid status codes. >> >> Locally, on top of this patch, I've run: >> >> jtreg -jdk:build/macosx-x86_64-server-release/images/jdk -a -ea -esa >> -agentvm -conc:4 -ignore:quiet test/jdk/java/net/httpclient >> >> and all tests have passed: >> >> Test results: passed: 190 >> >> Could I please get a review of this patch and someone to sponsor it? >> >> [1] https://tools.ietf.org/html/rfc2616#section-6.1.1 >> >> -Jaikiran >> >
