Re: JEP-353 - Socket.connect now throws NoRouteToHostException as against ConnectException previously
On 24/08/2021 04:09, Jaikiran Pai wrote: Do you connections to the Apache HTTP client library and the retry code that is looking for specific exceptions? From a distance it seems very fragile and depending on very implementation specific behavior. I wonder if it has ever been tested on Windows or with an untimed connect. I am not involved in the Apache HTTP client library project. However, I will go ahead and open a discussion in their mailing list and bring this issue to their attention, so that they can decide how to deal with it. Thank you for your help and the explanation. This has now been fixed in the Apache HTTP client library to no longer treat these two exception types differently when it comes to retry handling logic Good. Note that BindException is possible when attempt to establish a connection because the kernel will bind the socket to a local port if not explicitly bound already. It might arise when there are no ports available. I don't know if this changes the retry logic in the HTTP client but thought I should mention it. -Alan
RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
Collections.sort is just a wrapper, so it is better to use an instance method directly. - Commit messages: - [PATCH] Replace usages of Collections.sort with List.sort call in public java modules Changes: https://git.openjdk.java.net/jdk/pull/5229/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5229&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272863 Stats: 27 lines in 10 files changed: 0 ins; 8 del; 19 mod Patch: https://git.openjdk.java.net/jdk/pull/5229.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5229/head:pull/5229 PR: https://git.openjdk.java.net/jdk/pull/5229
Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules
On Mon, 23 Aug 2021 21:01:48 GMT, Andrey Turbanov wrote: > Collections.sort is just a wrapper, so it is better to use an instance method > directly. The changes in the src/java.desktop/ looks fine. Filed: https://bugs.openjdk.java.net/browse/JDK-8272863 - Marked as reviewed by serb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5229
Re: JEP-353 - Socket.connect now throws NoRouteToHostException as against ConnectException previously
Do you connections to the Apache HTTP client library and the retry code that is looking for specific exceptions? From a distance it seems very fragile and depending on very implementation specific behavior. I wonder if it has ever been tested on Windows or with an untimed connect. I am not involved in the Apache HTTP client library project. However, I will go ahead and open a discussion in their mailing list and bring this issue to their attention, so that they can decide how to deal with it. Thank you for your help and the explanation. This has now been fixed in the Apache HTTP client library to no longer treat these two exception types differently when it comes to retry handling logic https://github.com/apache/httpcomponents-client/pull/311 -Jaikiran
Re: RFR: 8272805: Avoid looking up standard charsets [v2]
On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov wrote:
>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>>
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>>
>> This could be done more efficiently(up to x20 time faster) with use of
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>>
>> The later variant also makes the code cleaner, as it is known not to throw
>> UnsupportedEncodingException in contrary to the former variant.
>>
>> This change includes:
>> * demo/utils
>> * jdk.xx packages
>> * Some places were missed in the previous changes. I have found it by
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and
>> desktop tests.
>>
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>>
>> Code excluded in this fix: the Xerces library(should be fixed upstream),
>> J2DBench(should be compatible to 1.4), some code in the network(the change
>> there are not straightforward, will do it later).
>>
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 14 additional
> commits since the last revision:
>
> - Update the usage of Files.readAllLines()
> - Update datatransfer
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Fix related imports
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Cleanup UnsupportedEncodingException
> - Update PacketStream.java
> - Rollback TextTests, should be compatible with jdk1.4
> - Rollback TextRenderTests, should be compatible with jdk1.4
> - ... and 4 more:
> https://git.openjdk.java.net/jdk/compare/aaa7beaf...e7127644
Marked as reviewed by turban...@github.com (no known OpenJDK username).
-
PR: https://git.openjdk.java.net/jdk/pull/5210
Re: RFR: 8272805: Avoid looking up standard charsets
On Sun, 22 Aug 2021 18:31:02 GMT, Andrey Turbanov wrote: > I think it's worth to update _static_ initializer in > `sun.datatransfer.DataFlavorUtil.CharsetComparator` too. Updated as suggested. - PR: https://git.openjdk.java.net/jdk/pull/5210
Re: RFR: 8272805: Avoid looking up standard charsets [v2]
On Sun, 22 Aug 2021 15:09:26 GMT, Alan Bateman wrote:
>> Sergey Bylokhov has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 14 additional
>> commits since the last revision:
>>
>> - Update the usage of Files.readAllLines()
>> - Update datatransfer
>> - Merge branch 'master' into standard-encodings-in-non-public-modules
>> - Merge branch 'master' into standard-encodings-in-non-public-modules
>> - Fix related imports
>> - Merge branch 'master' into standard-encodings-in-non-public-modules
>> - Cleanup UnsupportedEncodingException
>> - Update PacketStream.java
>> - Rollback TextTests, should be compatible with jdk1.4
>> - Rollback TextRenderTests, should be compatible with jdk1.4
>> - ... and 4 more:
>> https://git.openjdk.java.net/jdk/compare/465eb90c...e7127644
>
> src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line
> 342:
>
>> 340:
>> 341: try {
>> 342: for (String line : Files.readAllLines(statusPath, UTF_8)) {
>
> The 1-arg readAllLines is specified to use UTF-8 so you can drop the second
> parameter here if you want.
Thank you for the suggestion, I have fixed this and a few other places as well.
-
PR: https://git.openjdk.java.net/jdk/pull/5210
Re: RFR: 8270290: NTLM authentication fails if HEAD request is used
Hi, On 8/13/21, Michael McMahon wrote: > Hi, > > A question about this issue. Can you explain why the server/proxy is > sending a response body to a HEAD request? > > My reading of the RFCs suggests this is not allowed. Thanks for your comment and sorry for the late reply. To put aside the question about the support for non-compliant proxy servers, consider the scenario with HTTPS tunneling where proxy server never sees the HEAD request (it receives CONNECT). Please see the following abridged interaction where HEAD request is initiated from java code to HTTPS host some.hostname.com with proxy enabled: Transmission Control Protocol, Src Port: 53335, Dst Port: 8080, Seq: 1, Ack: 1, Len: 185 Hypertext Transfer Protocol CONNECT some.hostname.com:443 HTTP/1.1\r\n User-Agent: Java/1.8.0_302\r\n Host: some.hostname.com\r\n Proxy-Connection: keep-alive\r\n \r\n Transmission Control Protocol, Src Port: 8080, Dst Port: 53335, Seq: 7245, Ack: 186, Len: 413 Hypertext Transfer Protocol HTTP/1.1 407 Proxy Authentication Required\r\n Proxy-Authenticate: NTLM\r\n Proxy-Connection: close\r\n Connection: close\r\n Content-Length: 7384\r\n \r\n File Data: 7384 bytes Line-based text data: text/html (39 lines) \r\n [...] Transmission Control Protocol, Src Port: 53336, Dst Port: 8080, Seq: 1, Ack: 1, Len: 281 Hypertext Transfer Protocol CONNECT some.hostname.com:443 HTTP/1.1\r\n User-Agent: Java/1.8.0_302\r\n Host: some.hostname.com\r\n Proxy-Connection: keep-alive\r\n Proxy-authorization: NTLM TlRM[...]\r\n \r\n Transmission Control Protocol, Src Port: 8080, Dst Port: 53336, Seq: 7245, Ack: 282, Len: 690 Hypertext Transfer Protocol HTTP/1.1 407 Proxy Authentication Required\r\n Proxy-Authenticate: NTLM TlRM[...]\r\n Proxy-Connection: Keep-Alive\r\n Connection: Keep-Alive\r\n Content-Length: 7401\r\n \r\n File Data: 7401 bytes Line-based text data: text/html (39 lines) \r\n [...] Transmission Control Protocol, Src Port: 53336, Dst Port: 8080, Seq: 282, Ack: 7935, Len: 781 Hypertext Transfer Protocol CONNECT some.hostname.com:443 HTTP/1.1\r\n User-Agent: Java/1.8.0_302\r\n Host: some.hostname.com\r\n Proxy-Connection: keep-alive\r\n Proxy-authorization: NTLM TlRML[...]\r\n \r\n Transmission Control Protocol, Src Port: 8080, Dst Port: 53336, Seq: 7935, Ack: 1063, Len: 39 Hypertext Transfer Protocol HTTP/1.1 200 Connection established\r\n \r\n In this case the response code from "200 Connection established" response cannot be read by jdk because response body from the last CONNECT response was not fully read from the socket, jdk master will fail with the following trace: java.util.NoSuchElementException at java.base/java.util.StringTokenizer.nextToken(StringTokenizer.java:347) at java.base/sun.net.www.protocol.http.HttpURLConnection.doTunneling0(HttpURLConnection.java:2191) [...] This can be reproduced running NTLMHeadTest.java with TUNNEL argument. SERVER and PROXY modes were added to the test for completeness, it may be better to remove them. > > [...] > >> PR: https://git.openjdk.java.net/jdk/pull/4753 > -- -Alex
Re: RFR: 8253178: Replace LinkedList Impl in net.http.FilterFactory [v3]
On Mon, 19 Jul 2021 10:24:27 GMT, Sergei Ustimenko wrote: >> This patch replaces a LinkedList data structure used in the >> net.http.FilterFactory class with an ArrayList. This issue relates to >> [JDK-8246048: Replace LinkedList with ArrayLists in >> java.net.](https://bugs.openjdk.java.net/browse/JDK-8246048). >> >> The list created once per HttpClient and filled with upfront known values (3 >> of them in the jdk.internal.net.http.HttpClientImpl#initFilters: >> AuthenticationFilter.class, RedirectFilter.class and depending on the >> presence of a cookieHandler - a CookieFilter.class). > > Sergei Ustimenko has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains two additional > commits since the last revision: > > - 8253178: Trim the size for list with headers > - 8253178: Replace LinkedList Impl in net.http.FilterFactory LGTM. I can sponsor once you have integrated . - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4721
Re: RFR: 8272805: Avoid looking up standard charsets [v2]
On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov wrote:
>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>>
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>>
>> This could be done more efficiently(up to x20 time faster) with use of
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>>
>> The later variant also makes the code cleaner, as it is known not to throw
>> UnsupportedEncodingException in contrary to the former variant.
>>
>> This change includes:
>> * demo/utils
>> * jdk.xx packages
>> * Some places were missed in the previous changes. I have found it by
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and
>> desktop tests.
>>
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>>
>> Code excluded in this fix: the Xerces library(should be fixed upstream),
>> J2DBench(should be compatible to 1.4), some code in the network(the change
>> there are not straightforward, will do it later).
>>
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 14 additional
> commits since the last revision:
>
> - Update the usage of Files.readAllLines()
> - Update datatransfer
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Fix related imports
> - Merge branch 'master' into standard-encodings-in-non-public-modules
> - Cleanup UnsupportedEncodingException
> - Update PacketStream.java
> - Rollback TextTests, should be compatible with jdk1.4
> - Rollback TextRenderTests, should be compatible with jdk1.4
> - ... and 4 more:
> https://git.openjdk.java.net/jdk/compare/db47f6e1...e7127644
Changes to http server look good to me.
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5210
