Re: RFR 8245245 : WebSocket can loose the URL encoding of URI query parameters

2020-06-29 Thread Chris Hegarty
> On 26 Jun 2020, at 18:18, Daniel Fuchs wrote: > > I concur. Rahul has convinced me. > Rahul also pointed me to a test that verifies that the IAE is > thrown, so I believe that > http://cr.openjdk.java.net/~ryadav/webrev_8245245/index.html LGTM. -Chris.

Re: 8248703 Clarify the behavior of java.net.NetworkInterface::equals

2020-07-02 Thread Chris Hegarty
> On 2 Jul 2020, at 15:32, Daniel Fuchs wrote: > > ... > > Webrev updated: > http://cr.openjdk.java.net/~dfuchs/webrev_8248703/webrev.01/ > > Is this still appropriate to push to 15 or should I rather aim at 16? LGTM. I don’t have a strong opinion which release this goes into. -Chris.

Re: 8248865: Document JNDI/LDAP timeout properties

2020-07-07 Thread Chris Hegarty
Looks ok to me. -Chris. > On 6 Jul 2020, at 16:39, Daniel Fuchs wrote: > > Hi, > > Please find below a doc only change to document two > JDK specific JNDI/LDAP timeout properties: > > 8248865: Document JNDI/LDAP timeout properties > https://bugs.openjdk.java.net/browse/JDK-8248865 > > webrev

Re: 8249786: java/net/httpclient/websocket/PendingPingTextClose.java fails very infrequently

2020-08-07 Thread Chris Hegarty
Daniel, > On 23 Jul 2020, at 16:02, Daniel Fuchs wrote: > > Hi, > > More testing revealed that some other tests of the same family > kept on failing intermittently, though my changes to > PendingOperation.java should have fixed them. > > So here is a broader fix - which seems to have fixed th

Re: 8229822: ThrowingPushPromises tests sometimes fail due to EOF

2020-08-07 Thread Chris Hegarty
Daniel, > On 31 Jul 2020, at 15:56, Daniel Fuchs wrote: > > Hi, > > Please find below a fix for: > > 8229822: ThrowingPushPromises tests sometimes fail due to EOF > https://bugs.openjdk.java.net/browse/JDK-8229822 > > While trying to write a good test for JDK-8245462 I stumbled > on two issue

Re: RFR 8248006 : Revisit exceptions thrown when creating an HttpClient fails due to unavailability of underlying resources.

2020-08-07 Thread Chris Hegarty
Rahul, > On 7 Aug 2020, at 10:09, Rahul Yadav wrote: > > Hello, > > ... > Issue: https://bugs.openjdk.java.net/browse/JDK-8248006 > webrev: http://cr.openjdk.java.net/~ryadav/webrev_8248006/index.html > csr : https://bugs.openjdk.java.net/browse/JDK-8251198 > Looks good to me. I’m happy t

Re: RFR[8189744]: 'Deprecate the JDK-specific API for setting socket options, jdk.net.Sockets’

2020-08-13 Thread Chris Hegarty
> On 13 Aug 2020, at 14:08, Patrick Concannon > wrote: > > Hi Daniel, > > Thanks for taking a look. > > Here is a link to the CSR: https://bugs.openjdk.java.net/browse/JDK-8251532 > LGTM. -Chris.

Re: RFR[8246047]: 'Replace LinkedList impl in net.http.websocket.BuilderImpl'

2020-08-14 Thread Chris Hegarty
> On 14 Aug 2020, at 16:48, Conor Cleary wrote: > > Thanks for the suggestion Daniel, looks much nicer without the casting in the > immutableCopy function as well! > > I made a new patch with those reverted changes and it looks much nicer and > still behaves as expected. > > webrev: > htt

Re: RFR 8251715 : Throw UncheckedIOException in place of InternalError when HttpClient fails due to unavailability of underlying resources required by SSLContext.

2020-08-18 Thread Chris Hegarty
Rahul, > On 18 Aug 2020, at 10:21, Rahul Yadav wrote: > > ... > > Issue: https://bugs.openjdk.java.net/browse/JDK-8251715 > webrev: http://cr.openjdk.java.net/~ryadav/webrev_8251715/index.html > This looks good. In the test, i see that you have used expectThrows. expectThrows returns the ex

Re: RFR 8251715 : Throw UncheckedIOException in place of InternalError when HttpClient fails due to unavailability of underlying resources required by SSLContext.

2020-08-18 Thread Chris Hegarty
v ), and print the exception cause in the error message (if not the excepted type). No need for further reviews from me. -Chris. > - rahul > > On 18/08/2020 14:28, Chris Hegarty wrote: >> Rahul, >> >>> On 18 Aug 2020, at 10:21, Rahul Yadav wrote: >>> >>

Re: 8245462: HttpClient send throws InterruptedException when interrupted but does not cancel request

2020-08-27 Thread Chris Hegarty
Daniel, > On 14 Aug 2020, at 16:43, Daniel Fuchs wrote: > > Hi, > > After discussion with Chris & Alan I have slightly reworded > the API documentation, and ensured the MultiExchange is only > weakly referenced from the MinimalFuture to avoid the risk of > pinning the exchange in memory after i

Re: RFR: 8252767: URLConnection.setRequestProperty throws IllegalAccessError [v3]

2020-09-07 Thread Chris Hegarty
On Sun, 6 Sep 2020 11:21:58 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessEr

Re: RFR: 8252767: URLConnection.setRequestProperty throws IllegalAccessError [v4]

2020-09-07 Thread Chris Hegarty
On Mon, 7 Sep 2020 09:15:39 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessEr

Re: RFR: 8252767: URLConnection.setRequestProperty throws IllegalAccessError [v4]

2020-09-07 Thread Chris Hegarty
On Mon, 7 Sep 2020 09:35:16 GMT, Jaikiran Pai wrote: >>> I see Jaikiran has used "/integrate" but I don't think the review is >>> complete yet. The change may be trivial but it's >>> really important to give others a chance to comment if they wish. >> >> Sorry, I'm new this whole flow. I saw th

Re: RFR: 8252767: URLConnection.setRequestProperty throws IllegalAccessError [v5]

2020-09-07 Thread Chris Hegarty
On Mon, 7 Sep 2020 11:04:35 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessEr

Re: RFR: 8252767: URLConnection.setRequestProperty throws IllegalAccessError [v6]

2020-09-07 Thread Chris Hegarty
On Mon, 7 Sep 2020 11:47:36 GMT, Jaikiran Pai wrote: >> Can I please get a review and a sponsor for a fix for the issue reported at >> https://bugs.openjdk.java.net/browse/JDK-8252767? >> As noted in that issue, the `sun.net.www.URLConnection#setRequestProperty` >> is throwing a `IllegalAccessEr

Re: RFR: 8251496: Fix doclint warnings in jdk.net.httpserver [v3]

2020-09-09 Thread Chris Hegarty
On Wed, 9 Sep 2020 13:29:05 GMT, Roger Riggs wrote: >> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8251496: reworded abstract cstr comments; removed unnecessary punctuation > > The @param and @return lines that star

Re: RFR: 8251496: Fix doclint warnings in jdk.net.httpserver [v2]

2020-09-09 Thread Chris Hegarty
On Wed, 9 Sep 2020 08:50:38 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my doc-only fix for JDK-8251496 - ‘Fix doclint >> warnings in jdk.net.httpserver’ ? >> >> This fix addresses the warnings generated by `javadoc -Xdoclint` due to >> missing/incomplete API docum

Re: Thread safety problem in java.net.ProxySelector

2020-09-09 Thread Chris Hegarty
Seems like a bug. Can you please file an issue for it. Thanks, -Chris. > On 2 Sep 2020, at 14:16, David Lloyd wrote: > > The default proxy selector field in java.net.ProxySelector is > essentially a global variable with no thread safety measures taken to > ensure that accesses are valid. Anecd

Re: RFR: 8251496: Fix doclint warnings in jdk.net.httpserver [v4]

2020-09-10 Thread Chris Hegarty
On Thu, 10 Sep 2020 11:43:51 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my doc-only fix for JDK-8251496 - ‘Fix doclint >> warnings in jdk.net.httpserver’ ? >> >> This fix addresses the warnings generated by `javadoc -Xdoclint` due to >> missing/incomplete API docu

Re: RFR: 8245309: Re-examine use of ThreadLocalCoders in sun.net.www.ParseUtil [v2]

2020-09-16 Thread Chris Hegarty
On Tue, 15 Sep 2020 15:43:59 GMT, Julia Boes wrote: >> Replaced the use of ThreadLocalCoders with regular non-caching >> CharsetEncoder and added a benchmark to confirm that >> there is no performance impact. > > Julia Boes has updated the pull request incrementally with one additional > commit

Re: RFR: 8245309: Re-examine use of ThreadLocalCoders in sun.net.www.ParseUtil [v3]

2020-09-16 Thread Chris Hegarty
On Wed, 16 Sep 2020 09:27:01 GMT, Julia Boes wrote: >> Replaced the use of ThreadLocalCoders with regular non-caching >> CharsetEncoder and added a benchmark to confirm that >> there is no performance impact. > > Julia Boes has updated the pull request incrementally with one additional > commit

Re: RFR: 8252374: Add a new factory method to concatenate a sequence of BodyPublisher instances into a single publisher. [v4]

2020-09-18 Thread Chris Hegarty
On Fri, 18 Sep 2020 09:01:56 GMT, Daniel Fuchs wrote: >> Continuing the review with a PR... >> >> 8252374: Add a new factory method to concatenate a sequence >> of BodyPublisher instances into a single publisher. >> https://bugs.openjdk.java.net/browse/JDK-8252374 >> >> >> Draft CSR:

Re: RFR: 8252374: Add a new factory method to concatenate a sequence of BodyPublisher instances into a single publisher.

2020-09-22 Thread Chris Hegarty
On Mon, 7 Sep 2020 13:54:52 GMT, Daniel Fuchs wrote: > Continuing the review with a PR... > > 8252374: Add a new factory method to concatenate a sequence > of BodyPublisher instances into a single publisher. > https://bugs.openjdk.java.net/browse/JDK-8252374 > > > Draft CSR: > https:/

Re: RFR: 8253053: Javadoc clean up in Authenticator and BasicAuthenicator [v2]

2020-09-23 Thread Chris Hegarty
On Tue, 22 Sep 2020 16:20:17 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my doc-only fix for JDK-8253053 - 'Javadoc clean >> up in Authenticator and BasicAuthenicator' >> ? >> This fix is set of formatting changes intended to clean up the javadoc of >> the following

Re: RFR: 8253053: Javadoc clean up in Authenticator and BasicAuthenicator [v3]

2020-09-26 Thread Chris Hegarty
On Fri, 25 Sep 2020 17:32:41 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my doc-only fix for JDK-8253053 - 'Javadoc clean >> up in Authenticator and BasicAuthenicator' >> ? >> This fix is set of formatting changes intended to clean up the javadoc of >> the following

Re: RFR: 8252374: Add a new factory method to concatenate a sequence of BodyPublisher instances into a single publisher. [v6]

2020-10-05 Thread Chris Hegarty
On Fri, 18 Sep 2020 16:41:43 GMT, Daniel Fuchs wrote: >> Continuing the review with a PR... >> >> 8252374: Add a new factory method to concatenate a sequence >> of BodyPublisher instances into a single publisher. >> https://bugs.openjdk.java.net/browse/JDK-8252374 >> >> >> Draft CSR:

Re: RFR: 8253179: Replace LinkedList Impl in net.http.Http2Connection [v2]

2020-10-06 Thread Chris Hegarty
On Fri, 2 Oct 2020 08:41:56 GMT, Conor Cleary wrote: >> This patch replaces a LinkedList data structure used in the >> net.http.Http2Connection class with an ArrayList. This >> issue relates to [JDK-8246048: Replace LinkedList with ArrayLists in >> java.net](https://bugs.openjdk.java.net/browse/

Re: RFR: 8252374: Add a new factory method to concatenate a sequence of BodyPublisher instances into a single publisher. [v10]

2020-10-07 Thread Chris Hegarty
On Tue, 6 Oct 2020 17:03:28 GMT, Daniel Fuchs wrote: >> Continuing the review with a PR... >> >> 8252374: Add a new factory method to concatenate a sequence >> of BodyPublisher instances into a single publisher. >> https://bugs.openjdk.java.net/browse/JDK-8252374 >> >> >> Draft CSR: >

Re: RFR: 8252374: Add a new factory method to concatenate a sequence of BodyPublisher instances into a single publisher. [v11]

2020-10-09 Thread Chris Hegarty
On Wed, 7 Oct 2020 13:12:33 GMT, Daniel Fuchs wrote: >> Continuing the review with a PR... >> >> 8252374: Add a new factory method to concatenate a sequence >> of BodyPublisher instances into a single publisher. >> https://bugs.openjdk.java.net/browse/JDK-8252374 >> >> >> Draft CSR: >

Re: RFR: 8229867: Re-examine synchronization usages in http and https protocol handlers

2020-10-09 Thread Chris Hegarty
On Thu, 8 Oct 2020 11:22:09 GMT, Daniel Fuchs wrote: > Hi, > > This is a fix that upgrades the old HTTP and HTTPS legacy stack to use > virtual-thread friendly locking instead of > synchronized monitors. > Most of the changes are mechanical - but there are still a numbers of subtle > non-mecha

Re: RFR: 8245194: Unix domain socket channel implementation [v17]

2020-10-09 Thread Chris Hegarty
On Tue, 6 Oct 2020 19:53:18 GMT, Michael McMahon wrote: >> Continuing this review as a PR on github with the comments below >> incorporated. I expect there will be a few more >> iterations before integrating. >> On 06/09/2020 19:47, Alan Bateman wrote: >>> On 26/08/2020 15:24, Michael McMahon wr

Re: RFR: 8229867: Re-examine synchronization usages in http and https protocol handlers [v3]

2020-10-13 Thread Chris Hegarty
On Mon, 12 Oct 2020 13:50:30 GMT, Daniel Fuchs wrote: >> Hi, >> >> This is a fix that upgrades the old HTTP and HTTPS legacy stack to use >> virtual-thread friendly locking instead of >> synchronized monitors. >> Most of the changes are mechanical - but there are still a numbers of subtle >> n

Re: RFR: 8254704: Add missing @since tag to BodyPublishers::concat

2020-10-13 Thread Chris Hegarty
On Tue, 13 Oct 2020 15:22:07 GMT, Daniel Fuchs wrote: > Please review a trivial change that adds a missing `@since 16` tag to the API > documentation of the new > `BodyPublishers::concat` method. I missed that in my changes that was > integrated yesterday - and that was unfortunately > missed i

Re: Request: mechanism for making HttpRequest.Builder from HttpRequest

2020-10-15 Thread Chris Hegarty
Hi Craig, > On 15 Oct 2020, at 17:22, Craig Andrews wrote: > > Java's HttpClient relies on the builder pattern to produce immutable objects > on which actual operations are done. However, it currently lacks a way to > convert those working objects back to builders in order to mutate them. I a

Re: RFR: 8245194: Unix domain socket channel implementation [v32]

2020-10-21 Thread Chris Hegarty
On Tue, 20 Oct 2020 14:21:42 GMT, Michael McMahon wrote: >> Continuing this review as a PR on github with the comments below >> incorporated. I expect there will be a few more iterations before >> integrating. >> >> On 06/09/2020 19:47, Alan Bateman wrote: >>> On 26/08/2020 15:24, Michael McMa

Re: RFR: 8245194: Unix domain socket channel implementation [v34]

2020-10-21 Thread Chris Hegarty
On Wed, 21 Oct 2020 16:31:06 GMT, Michael McMahon wrote: >> Continuing this review as a PR on github with the comments below >> incorporated. I expect there will be a few more iterations before >> integrating. >> >> On 06/09/2020 19:47, Alan Bateman wrote: >>> On 26/08/2020 15:24, Michael McMa

Re: RFR: 8245194: Unix domain socket channel implementation [v34]

2020-10-21 Thread Chris Hegarty
On Wed, 21 Oct 2020 16:31:06 GMT, Michael McMahon wrote: >> Continuing this review as a PR on github with the comments below >> incorporated. I expect there will be a few more iterations before >> integrating. >> >> On 06/09/2020 19:47, Alan Bateman wrote: >>> On 26/08/2020 15:24, Michael McMa

Re: RFR: 8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonit… [v2]

2020-10-21 Thread Chris Hegarty
On Wed, 21 Oct 2020 16:45:27 GMT, Jan Lahoda wrote: >> …orStateException: current thread is not owner >> >> The KeepAliveStreamCleaner is calling Condition.wait (i.e. the Object.wait >> variant), should presumably be calling Condition.await. (NetBeans is >> producing warnings on the line as we

Re: RFR: 8245194: Unix domain socket channel implementation [v35]

2020-10-22 Thread Chris Hegarty
On Thu, 22 Oct 2020 08:14:31 GMT, Michael McMahon wrote: >> Continuing this review as a PR on github with the comments below >> incorporated. I expect there will be a few more iterations before >> integrating. >> >> On 06/09/2020 19:47, Alan Bateman wrote: >>> On 26/08/2020 15:24, Michael McMa

Re: RFR: 8255405: sun/net/ftp/imp/FtpClient uses SimpleDateFormat in not thread-safe manner

2020-10-27 Thread Chris Hegarty
On Mon, 26 Oct 2020 16:51:25 GMT, Igor Ignatyev wrote: > Hi all, > > could you please review this small and trivial fix? > > `sun/net/ftp/imp/FtpClient::dateFormats` is an array of `SimpleDateFormat` > which are shared among all instances of `FtpClient`. the fact that > `SimpleDateFormat` isn

Re: RFR: 8255405: sun/net/ftp/imp/FtpClient uses SimpleDateFormat in not thread-safe manner

2020-10-27 Thread Chris Hegarty
On Mon, 26 Oct 2020 16:51:25 GMT, Igor Ignatyev wrote: > Hi all, > > could you please review this small and trivial fix? > > `sun/net/ftp/imp/FtpClient::dateFormats` is an array of `SimpleDateFormat` > which are shared among all instances of `FtpClient`. the fact that > `SimpleDateFormat` isn

Re: RFR: 8255405: sun/net/ftp/imp/FtpClient uses SimpleDateFormat in not thread-safe manner [v3]

2020-10-28 Thread Chris Hegarty
On Wed, 28 Oct 2020 15:25:06 GMT, Igor Ignatyev wrote: >> Hi all, >> >> could you please review this small and trivial fix? >> >> `sun/net/ftp/imp/FtpClient::dateFormats` is an array of `SimpleDateFormat` >> which are shared among all instances of `FtpClient`. the fact that >> `SimpleDateForm

Re: RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders`

2020-11-02 Thread Chris Hegarty
On Mon, 2 Nov 2020 17:14:28 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my fix for JDK-8253005: 'Add `@throws > IOException` in javadoc for `HttpEchange.sendResponseHeaders`' ? > > The method `HttpEchange.sendResponseHeaders` throws an `IOException` but is > unspecifi

Re: RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders`

2020-11-03 Thread Chris Hegarty
On Mon, 2 Nov 2020 18:35:21 GMT, Patrick Concannon wrote: >> Marked as reviewed by chegar (Reviewer). > >> /csr needed > > Link to CSR: https://bugs.openjdk.java.net/browse/JDK-8255778 Is there a test that asserts this: "if the response headers have already been sent" ? - PR: ht

Re: RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders` [v3]

2020-11-04 Thread Chris Hegarty
On Wed, 4 Nov 2020 12:16:09 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my fix for JDK-8253005: 'Add `@throws >> IOException` in javadoc for `HttpEchange.sendResponseHeaders`' ? >> >> The method `HttpEchange.sendResponseHeaders` throws an `IOException` but is >> un

Re: RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest

2020-11-04 Thread Chris Hegarty
On Wed, 4 Nov 2020 14:51:07 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review our code for JDK-8252304: 'Seed an > HttpRequest.Builder from an existing HttpRequest'? > > This RFR proposes a new factory method for creating a new `HttpRequest` > builder from an existing `Http

Re: RFR: 8246741: NetworkInterface/UniqueMacAddressesTest: mac address uniqueness test failed

2020-11-05 Thread Chris Hegarty
On Thu, 5 Nov 2020 12:46:08 GMT, Conor Cleary wrote: > The primary goal of this fix was to refactor > NetworkInterface/UniqueMacAddressesTest.java to make use of the > jdk.test.lib.NetworkConfiguration class instead of java.net.NetworkInterface. > This was due to the original failure being rel

Re: RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v3]

2020-11-05 Thread Chris Hegarty
On Thu, 5 Nov 2020 16:23:13 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review our code for JDK-8252304: 'Seed an >> HttpRequest.Builder from an existing HttpRequest'? >> >> This RFR proposes a new factory method for creating a new `HttpRequest` >> builder from an existin

Re: RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders` [v7]

2020-11-05 Thread Chris Hegarty
On Thu, 5 Nov 2020 17:08:10 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my fix for JDK-8253005: 'Add `@throws >> IOException` in javadoc for `HttpEchange.sendResponseHeaders`' ? >> >> The method `HttpEchange.sendResponseHeaders` throws an `IOException` but is >> un

Re: RFR: 8253005: Add `@throws IOException` in javadoc for `HttpEchange.sendResponseHeaders` [v7]

2020-11-06 Thread Chris Hegarty
On Thu, 5 Nov 2020 17:56:00 GMT, Daniel Fuchs wrote: >> test/jdk/java/net/httpclient/SendResponseHeadersTest.java line 99: >> >>> 97: // unexpected exception thrown, return error to >>> client >>> 98: t.printStackTrace(); >>> 99: os.wr

Re: RFR: 8255244: HttpClient: Response headers contain incorrectly encoded Unicode characters

2020-11-13 Thread Chris Hegarty
On Wed, 11 Nov 2020 16:45:49 GMT, Daniel Fuchs wrote: > The HTTP/1.1 Header Parser of the new HttpClient currently assumes that all > headers (names and value) are US-ASCII and as a result mis-decode any byte > whose value is > 127; For instance, 0x80 (128) gets decoded as a U+FF80 > (65408) i

Re: RFR: 8252304: Seed an HttpRequest.Builder from an existing HttpRequest [v8]

2020-11-17 Thread Chris Hegarty
On Mon, 16 Nov 2020 15:54:18 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review our code for JDK-8252304: 'Seed an >> HttpRequest.Builder from an existing HttpRequest'? >> >> This RFR proposes a new factory method for creating a new `HttpRequest` >> builder from an existi

Re: Use switch expressions in jdk.internal.net.http and java.net.http

2020-11-23 Thread Chris Hegarty
Hi Kartik, > On 21 Nov 2020, at 12:01, Kartik Ohri wrote: > > Hi! > I would like to submit this patch https://github.com/openjdk/jdk/pull/1364 > with the rationale to improve the > readability of the code. Can someone please take a look at it and crea

Re: RFR: 8255675: Typo in java.net.HttpURLConnection

2020-11-23 Thread Chris Hegarty
On Tue, 17 Nov 2020 07:50:10 GMT, ANUPAM DEV wrote: > 8255675: Typo in java.net.HttpURLConnection Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1250

Re: RFR: JDK-8257401: Use switch expressions in jdk.internal.net.http and java.net.http [v4]

2020-12-02 Thread Chris Hegarty
On Wed, 2 Dec 2020 09:42:09 GMT, Kartik Ohri wrote: >> Hi! >> Kindly review this patch to replace switch statements with switch >> expressions (where it makes sense) in the http client modules. The rationale >> is to improve readability of the code. >> Regards, >> Kartik > > Kartik Ohri has re

Re: RFR: 8255583: Investigate creating a test to trigger the condition in KeepAliveStreamCleaner [v3]

2020-12-09 Thread Chris Hegarty
On Mon, 7 Dec 2020 17:11:28 GMT, Conor Cleary wrote: >> The KeepAliveStreamCleaner in sun.net.ww.http package had been previously >> seen to fail with an IllegalMonitorStateException. This failure was caused >> by the use of `wait()` in a non synchronized block. This failure was >> mitigated t

Re: RFR: 8256459: java/net/httpclient/ManyRequests.java and java/net/httpclient/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information

2020-12-09 Thread Chris Hegarty
On Wed, 9 Dec 2020 15:37:42 GMT, Daniel Fuchs wrote: > Hi, > > Please find here a changeset that fixes the infrequent (but annoying) test > failures > caused by unexpected ConnectionException "Connection timed out: no further > information" > which have been observed to occur on some platform

Re: RFR: 8256459: java/net/httpclient/ManyRequests.java and java/net/httpclient/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information

2020-12-09 Thread Chris Hegarty
On Wed, 9 Dec 2020 15:37:42 GMT, Daniel Fuchs wrote: > Hi, > > Please find here a changeset that fixes the infrequent (but annoying) test > failures > caused by unexpected ConnectionException "Connection timed out: no further > information" > which have been observed to occur on some platform

Re: RFR: 8256459: java/net/httpclient/ManyRequests.java and java/net/httpclient/LineBodyHandlerTest.java fail infrequently with java.net.ConnectException: Connection timed out: no further information

2020-12-09 Thread Chris Hegarty
On Wed, 9 Dec 2020 18:50:50 GMT, Daniel Fuchs wrote: >> Hi, >> >> Please find here a changeset that fixes the infrequent (but annoying) test >> failures >> caused by unexpected ConnectionException "Connection timed out: no further >> information" >> which have been observed to occur on some p

Re: RFR: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens [v3]

2020-12-10 Thread Chris Hegarty
On Sat, 5 Dec 2020 04:54:27 GMT, Yasumasa Suenaga wrote: >> `InputStream` from `BodyPublishers.ofInputStream()` is usually closed when >> the stream reaches EOF. However IOE handler returns without closing. >> >> I confirmed this problem in `BodyPublishers.ofInputStream()`, but I think >> `Bod

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base

2020-12-15 Thread Chris Hegarty
On Sat, 31 Oct 2020 19:37:10 GMT, Stuart Marks wrote: >> I believe this changes is useful and still actual: >> 1. improve code to make it easier to read. >> 2. performance should be improved a bit too > > I’ll see if I can get somebody to take a look at this. This seems like a reasonable change,

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v3]

2020-12-16 Thread Chris Hegarty
On Wed, 16 Dec 2020 09:20:09 GMT, Andrey Turbanov wrote: >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8258422: Cleanup unnecess

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]

2020-12-17 Thread Chris Hegarty
On Wed, 16 Dec 2020 10:32:12 GMT, Andrey Turbanov wrote: >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8258422: Cleanup unnecess

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-17 Thread Chris Hegarty
Looping in a prior relevant issue in JIRA: https://bugs.openjdk.java.net/browse/JDK-8170568 - Improve address selection for network clients -Chris. > On 17 Dec 2020, at 13:45, Daniel Fuchs wrote: > > Hi Simone, > > We are investigating introducing a Service Provider interface > which would

Re: RFR: 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default executor when stopping.

2020-12-17 Thread Chris Hegarty
On Thu, 17 Dec 2020 14:51:44 GMT, Daniel Fuchs wrote: > Hi, > > Please find an almost trivial fix for: > 8258582: HttpClient: the HttpClient doesn't explicitly shutdown its default > executor when stopping. > > The HttpClient should shutdown his executor when stopping, when the executor > was

RFR: 8258696: Temporarily revert use of pattern match instanceof until docs-reference is fixed

2020-12-18 Thread Chris Hegarty
Temporarily revert use of pattern match instanceof construct until docs-reference is fixed, see JDK-8258657. ... Generating REFERENCE_API javadoc for 21 modules if (delegate instanceof ExecutorService service) { ^ (use --enab

Integrated: 8258696: Temporarily revert use of pattern match instanceof until docs-reference is fixed

2020-12-18 Thread Chris Hegarty
On Fri, 18 Dec 2020 16:54:59 GMT, Chris Hegarty wrote: > Temporarily revert use of pattern match instanceof construct until > docs-reference is fixed, see JDK-8258657. > > ... > Generating REFERENCE_API javadoc for 21 modules > > if (delegate instanceof Ex

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]

2020-12-19 Thread Chris Hegarty
On Thu, 17 Dec 2020 13:16:31 GMT, Aleksei Efimov wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8258422: Cleanup unnecessary null comparison before instanceof check in >> java.base >> take advantage of "flow

Re: [JDK-8257080] Java does not try all DNS results when opening a socket

2020-12-22 Thread Chris Hegarty
ect(String,int). -Chris. [1] https://bugs.openjdk.java.net/browse/JDK-8170568 [2] https://mail.openjdk.java.net/pipermail/net-dev/2019-April/012371.html [3] https://tools.ietf.org/html/rfc6555 > On 17 Dec 2020, at 14:39, Chris Hegarty wrote: > > Looping in a prior relevant issue i

Re: RFR: 8258422: Cleanup unnecessary null comparison before instanceof check in java.base [v4]

2021-01-08 Thread Chris Hegarty
On Fri, 8 Jan 2021 13:20:38 GMT, Aleksei Efimov wrote: >> This issue is blocked by [8258657][1]. It should not be integrated until >> after 8258657 has been resolved. The JIRA issues have been linked up to >> reflect this. >> >> [1]: https://bugs.openjdk.java.net/browse/JDK-8258657 > > [JDK-82

Re: RFR: 8259631: Reapply pattern match instanceof use in HttpClientImpl

2021-01-13 Thread Chris Hegarty
On Tue, 12 Jan 2021 16:05:01 GMT, Aleksei Efimov wrote: > Hi, > > The proposed change adds back [1] `instanceof` pattern match use to > `HttpClientImpl` class. It was previously removed by > [JDK-8258696](https://bugs.openjdk.java.net/browse/JDK-8258696) due to docs > build failure. > > Alek

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) [v3]

2021-01-14 Thread Chris Hegarty
On Thu, 14 Jan 2021 12:48:18 GMT, Сергей Цыпанов wrote: >> Instead of allocating a copy of underlying array via >> `CharArrayWriter.toCharArray()` and passing it to constructor of String >> String str = new String(charArrayWriter.toCharArray()); >> we could call `toString()` method >> String st

Re: RFR: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens [v5]

2021-01-14 Thread Chris Hegarty
On Thu, 14 Jan 2021 02:25:25 GMT, Yasumasa Suenaga wrote: >> `InputStream` from `BodyPublishers.ofInputStream()` is usually closed when >> the stream reaches EOF. However IOE handler returns without closing. >> >> I confirmed this problem in `BodyPublishers.ofInputStream()`, but I think >> `Bo

Re: RFR: 7146776: deadlock between URLStreamHandler.getHostAddress and file.Handler.openconnection [v2]

2021-01-18 Thread Chris Hegarty
On Sun, 17 Jan 2021 02:02:54 GMT, Jaikiran Pai wrote: >> As noted by the reporter in the comments of >> https://bugs.openjdk.java.net/browse/JDK-7146776, the `URLStreamHandler` in >> its `getHostAddress` is incorrectly synchronizing on the `URLStreamHandler` >> instance, instead of synchronizi

Re: RFR: JDK-8257235: [PATCH] InetAddress.isReachable: Try to use an IPPROTO_ICMP socket type before attempting RAW_SOCK [v2]

2021-01-18 Thread Chris Hegarty
On Wed, 23 Dec 2020 14:06:12 GMT, Jamie Le Tual wrote: >> Users have been able to send ICMP packets without the need for root >> privileges or the CAP_NET_RAW capability since at least kernel 3.11. >> >> For some time now, if the kernel parameter net.ipv4.ping_group_range >> included the gid

Re: RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL [v2]

2021-01-21 Thread Chris Hegarty
On Wed, 20 Jan 2021 15:46:13 GMT, Eirik Bjørsnøs wrote: >> By moving string splitting and concatenation into the canonizeString >> utility, we can defer allocation until we determine that canonization is >> required. This saves two string allocations and a string concat for the >> common case

Re: RFR: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL [v2]

2021-01-21 Thread Chris Hegarty
On Thu, 21 Jan 2021 09:48:50 GMT, Chris Hegarty wrote: >> Eirik Bjorsnos has updated the pull request incrementally with one >> additional commit since the last revision: >> >> As a part of the rename from "canonize" to "canonicalize", doCano

Re: RFR: JDK-8241995: Clarify InetSocketAddress::toString specification

2021-02-03 Thread Chris Hegarty
On Tue, 2 Feb 2021 15:55:40 GMT, Julia Boes wrote: > This change clarifies the InetSocketAddress::toString specification, which > was recently updated to reflect an implementation change [1]. The > specification is not changed, but it is enhanced with two examples and some > more verbiage, as

Re: RFR: 8235139: Remove the socket impl factory mechanism [v3]

2021-02-05 Thread Chris Hegarty
On Fri, 5 Feb 2021 11:25:57 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my proposed changeset for JDK-8235139: '`Remove >> the socket impl factory mechanism`' ? >> >> These changes propose to deprecate (for the eventual removal) the API points >> for statically con

Re: RFR: 8133686: HttpURLConnection.getHeaderFields and URLConnection.getRequestProperties methods return field values in reverse order [v2]

2021-02-08 Thread Chris Hegarty
On Fri, 29 Jan 2021 11:03:07 GMT, Evan Whelan wrote: >> Hi all, >> >> Please review this fix for which corrects the order in which field values >> are returned from the `HttpURLConnection.getHeaderFields` and >> `URLConnection.getRequestProperties` methods. >> >> Currently, the implementation

Re: RFR: 8133686: HttpURLConnection.getHeaderFields and URLConnection.getRequestProperties methods return field values in reverse order [v2]

2021-02-08 Thread Chris Hegarty
On Fri, 29 Jan 2021 10:59:20 GMT, Evan Whelan wrote: >> src/java.base/share/classes/java/net/URLConnection.java line 596: >> >>> 594: * corresponding values, they must be returned in the order they >>> were added, >>> 595: * preserving the insertion-order. >>> 596: * >> >> Here

Re: RFR: JDK-8261791: handleSendFailed in SctpChannelImpl.c potential leaks

2021-02-17 Thread Chris Hegarty
On Tue, 16 Feb 2021 12:26:54 GMT, Matthias Baesken wrote: > In another bug this question from me was answered by Alan Bateman : > > Btw. while adjusting Java_sun_nio_ch_sctp_SctpChannelImpl_receive0 , I > started to wonder what happens to the allocated memory in the same file in > handleSen

Re: RFR: JDK-8261791: handleSendFailed in SctpChannelImpl.c potential leaks

2021-02-17 Thread Chris Hegarty
On Wed, 17 Feb 2021 10:30:45 GMT, Chris Hegarty wrote: >> In another bug this question from me was answered by Alan Bateman : >> >> Btw. while adjusting Java_sun_nio_ch_sctp_SctpChannelImpl_receive0 , I >> started to wonder what happens to the allocated me

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

2021-02-18 Thread Chris Hegarty
On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов wrote: >> Non-static classes hold a link to their parent classes, which in many cases >> can be avoided. > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8261880: Remove s

Re: RFR: 8262027: Improve how HttpConnection detects a closed channel when taking/returning a connection to the pool [v5]

2021-02-23 Thread Chris Hegarty
On Mon, 22 Feb 2021 16:44:15 GMT, Daniel Fuchs wrote: >> Hi, >> >> Please find here a fix for: >> 8262027: Improve how HttpConnection detects a closed channel when >> taking/returning a connection to the pool >> >> While writing a new test to verify that it was possible to handle proxy >> *an

Re: RFR: 8262027: Improve how HttpConnection detects a closed channel when taking/returning a connection to the pool [v6]

2021-02-24 Thread Chris Hegarty
On Tue, 23 Feb 2021 17:20:13 GMT, Daniel Fuchs wrote: >> Hi, >> >> Please find here a fix for: >> 8262027: Improve how HttpConnection detects a closed channel when >> taking/returning a connection to the pool >> >> While writing a new test to verify that it was possible to handle proxy >> *an

RFR: 8262296 Fix remaining doclint warnings in jdk.httpserver

2021-02-24 Thread Chris Hegarty
Trivial clean up of remaining doclint warnings in jdk.httpserver. The minor spec clarifications do not amount to a normative change, so no CSR is required (they're documented the obvious and only possible behaviour). - Commit messages: - Initial changes Changes: https://git.openj

Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property)

2021-02-24 Thread Chris Hegarty
On Wed, 24 Feb 2021 15:54:29 GMT, Michael McMahon wrote: >> A number of net tests use a >> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)** >> to verify either the functionality of

Re: RFR: 8262296 Fix remaining doclint warnings in jdk.httpserver [v2]

2021-02-24 Thread Chris Hegarty
> Trivial clean up of remaining doclint warnings in jdk.httpserver. > > The minor spec clarifications do not amount to a normative change, so no CSR > is required (they're documented the obvious and only possible behaviour). Chris Hegarty has updated the pull request incre

Integrated: 8262296 Fix remaining doclint warnings in jdk.httpserver

2021-02-25 Thread Chris Hegarty
On Wed, 24 Feb 2021 13:58:38 GMT, Chris Hegarty wrote: > Trivial clean up of remaining doclint warnings in jdk.httpserver. > > The minor spec clarifications do not amount to a normative change, so no CSR > is required (they're documented the obvious and only possible behav

Re: RFR: JDK-8262430: doclint warnings in java.base module

2021-02-26 Thread Chris Hegarty
On Thu, 25 Feb 2021 22:59:23 GMT, Jonathan Gibbons wrote: > Please review some simple doc fixes in the `java.base` module. Two were > reported by doclint; the spelling error was detected by the IDE. Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/p

Re: RFR: 8262195: Harden tests that use the HostsFileNameService (jdk.net.hosts.file property) [v2]

2021-02-26 Thread Chris Hegarty
On Thu, 25 Feb 2021 15:38:02 GMT, Conor Cleary wrote: >> A number of net tests use a >> **[HostsFileNameService](https://github.com/openjdk/jdk/blob/382e38dd246596ec94a1f1ce0e0f9e87f53366c7/src/java.base/share/classes/java/net/InetAddress.java#L955)** >> to verify either the functionality of th

Re: RFR: 8253100: Fix "no comment" warnings in java.base/java.net [v3]

2021-02-26 Thread Chris Hegarty
On Fri, 26 Feb 2021 10:59:01 GMT, Daniel Fuchs wrote: >> Hi, >> >> Please find here a change that fixes "no comment" warnings generated by >> `javadoc -Xdoclint` for `java.base/java.net` > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last re

Re: RFR: 8252831: Correct "no comment" warnings in jdk.net module

2021-03-02 Thread Chris Hegarty
On Tue, 2 Mar 2021 10:15:13 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my changeset that fixes the "no comment" warnings > generated by `javadoc -Xdoclint` for `java.base/jdk.net`? > > Kind regards, > Patrick Marked as reviewed by chegar (Reviewer). - P

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable

2021-03-09 Thread Chris Hegarty
On Tue, 9 Mar 2021 11:07:07 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.net` > and `java.nio` packages to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick src/java.base/share/classes/java/net/Interfa

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v2]

2021-03-09 Thread Chris Hegarty
On Tue, 9 Mar 2021 17:59:27 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]

2021-03-10 Thread Chris Hegarty
On Tue, 9 Mar 2021 20:52:19 GMT, Mark Sheppard wrote: >> The disadvantage is that it would add another level of braces. > > maybe ... I find it would provide an easier flow to code ... an "easy on the > eye" call flow linkage - the variable is declared and then used instead > of the disc

Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]

2021-03-10 Thread Chris Hegarty
On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the

Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v2]

2021-03-15 Thread Chris Hegarty
On Mon, 15 Mar 2021 12:53:16 GMT, Jonathan Dowland wrote: >> This is an adaptation of a patch originally written by Shafi Ahmad in >> a comment on the JBS page but never submitted or merged. >> >> With -Xcheck:jni, the method java.net.NetworkInterface.getAll very >> quickly breaches the default

<    8   9   10   11   12   13   14   15   16   17   >