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. 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 :-) If I'm not mistaken you're a JDK author [1] right? 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) 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. 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
