Re: RFR: 8217237 HttpClient does not deal well with multi-valued WWW-Authenticate challenge headers

Hi Michael, I believe the code changes looks good. WRT to the test: 150 if (response.statusCode() != 200 && !response.body().equals(server.response())) { I believe you meant || not && best regards, -- daniel On 16/01/2019 16:46, Michael McMahon wrote: Could I get the following c

[13] RFR: 8216561: HttpClient: The logic of retry on connect exception is inverted

Hi, Please find below a small fix for: 8216561: HttpClient: The logic of retry on connect exception is inverted https://bugs.openjdk.java.net/browse/JDK-8216561 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8216561/webrev.01/ The patch now allows retry on connect exception, ensuring that t

Re: [13] RFR: 8216561: HttpClient: The logic of retry on connect exception is inverted

Hi Andrej, Yes that looks like my first implementation. But then I reflected that avoiding to map Optional to Duration and then back to a new Optional containing the same duration could be avoided by simply storing the original optional obtained from the HttpClient. The current code only creates

Re: [13] RFR: 8216561: HttpClient: The logic of retry on connect exception is inverted

Hi Andrej, On 18/01/2019 14:03, Andrej Golovnin wrote: Is creating new Optionals a real problem? And before you answer please remember what Donald Knuth said: The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times;

RFR: 8217264: HttpClient: Blocking operations in mapper function do not work as documented

Hi, Please find below a fix for: 8217264: HttpClient: Blocking operations in mapper function do not work as documented [1] https://bugs.openjdk.java.net/browse/JDK-8217264 webrev: [2] http://cr.openjdk.java.net/~dfuchs/webrev_8217264/webrev.00/ The issue here is that if you try to map

Re: java.net.http.HttpClient: invalid exception when bad status line is returned

Hi Dmitry, Thanks for reporting this. Right - the IllegalArgumentException is a bit strange there, and an IOException (or subclass of it) wrapping the NumberFormatException would be more appropriate. If you have OpenJDK author status and a JBS account then you could log a bug at https://bugs.

[13] RFR (doc): 8217627: HttpClient: The API documentation of BodySubscribers::mapping promotes bad behavior

Hi, Please find below a doc clarification change for: 8217627: HttpClient: The API documentation of BodySubscribers::mapping promotes bad behavior https://bugs.openjdk.java.net/browse/JDK-8217627 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8217627/webrev.00 New API doc: For bett

Re: RFR [13] 8217429: WebSocket over authenticating proxy fails to send Upgrade headers

Hi Chris, Looks good. I had the same question than Pavel about server.close(). No test for both proxy + server authorization with -Djdk.http.auth.tunneling.disabledSchemes ? cheers, -- daniel On 25/01/2019 14:21, Chris Hegarty wrote: When tunneling WebSocket over a proxy requiring authentica

Re: RFR [13] 8217429: WebSocket over authenticating proxy fails to send Upgrade headers

Thanks Chris. Makes sense, and looks good! best regards, -- daniel On 25/01/2019 17:07, Chris Hegarty wrote: Daniel, On 25 Jan 2019, at 15:34, Daniel Fuchs wrote: Hi Chris, Looks good. I had the same question than Pavel about server.close(). Answered already in reply to Pavel’s

RFR (testbug): 8210130: java/net/httpclient/UnknownBodyLengthTest.java failed

Hi, Please find below a test for: 8210130: java/net/httpclient/UnknownBodyLengthTest.java failed https://bugs.openjdk.java.net/browse/JDK-8210130 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8210130/webrev.00/ This test has been seen failing from time to time, often Caused by: java.io.IOE

RFR (testbug): 8217882: java/net/httpclient/MaxStreams.java failed once

Hi, Please find below a patch for: 8217882: java/net/httpclient/MaxStreams.java failed once https://bugs.openjdk.java.net/browse/JDK-8217882 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8217882/webrev.00/ The patch simply instruments the test with some more traces so that we might have a

RFR: (testbug) 8217903: java/net/httpclient/Response204.java fails with 404

Hi, Please find below a test for: 8217903: java/net/httpclient/Response204.java fails with 404 https://bugs.openjdk.java.net/browse/JDK-8217903 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8217903/webrev.00/ This is most certainly a port reuse issue. This test should be using the loopback

Re: RFR (testbug): 8217882: java/net/httpclient/MaxStreams.java failed once

28, 2019, at 9:42 AM, Daniel Fuchs <mailto:daniel.fu...@oracle.com>> wrote: Please find below a patch for: 8217882: java/net/httpclient/MaxStreams.java failed once https://bugs.openjdk.java.net/browse/JDK-8217882 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8217882/webrev.00/

Re: RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

Hi Chris, 116 List localList = List.copyOf(connections); I think it is still possible for CME to happen during the copyOf operation, even though it can reduce the window in which that might happen. The best would be to use CopyOnWriteArrayList instead of synchronizedList(). Otherwise

Re: RFR [13] 8217976: test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java fails intermittently

Looks good. While you're at it you could make: 123 CopyOnWriteArrayList connections; final. No need for a new webrev. best regards, -- daniel On 29/01/2019 12:53, Chris Hegarty wrote: Agreed. Should be good enough for test cleanup. http://cr.openjdk.java.net/~chegar/8217976/webrev.01/

RFR (testbug): 8216562: UnknownBodyLength sometimes fails due to "Connection reset by peer"

Hi, Please find below a fix for: 8216562: UnknownBodyLength sometimes fails due to "Connection reset by peer" https://bugs.openjdk.java.net/browse/JDK-8216562 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8216562/webrev.00/ My previous attempt at at fixing this test proved unsucce

RFR (testbug) 8218133: sun/net/www/protocol/http/ProtocolRedirect.java failed with "java.net.ConnectException"

8218133: sun/net/www/protocol/http/ProtocolRedirect.java failed with "java.net.ConnectException" https://bugs.openjdk.java.net/browse/JDK-8218133 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8218133/webrev.00/ Connection refused are usually a symptom of port reuse issue, and usi

Re: Question about com.sun.net.httpserver.HttpExchange#sendResponseHeaders

Hi Wembo, On 31/01/2019 23:56, Wenbo Zhu wrote: Per the HTTP spec, chunked encoding may not be needed if "Connection: close" is set and the TCP connection will be closed (FIN) to complete the response body. Are you referring to HTTP/1.0? https://tools.ietf.org/html/rfc1945#section-7.2.2 I am

Re: Question about com.sun.net.httpserver.HttpExchange#sendResponseHeaders

Hi Wembo, On 01/02/19 19:50, Wenbo Zhu wrote: 1) clarify in the API javadoc that chunked encoding is always applied even with Connection: close Chunked encoding is always applied if 0 is passed to sendResponseHeaders (this is a bit counter-intuitive, but 0 means chunked coding and -1 means Con

Re: HttpClient SSL Race Condition

Hi, It looks like this is JDK-8214418 - which has been fixed in 12.0.1 b03 and 13-ea b04. The issue was with the half closed semantics of the SSL engine in TLS 1.3. This is only loosely related to https://bugs.openjdk.java.net/browse/JDK-8217094 (we found that while testing the fix to JDK-821441

Re: HttpClient SSL Race Condition

On 03/02/2019 18:05, August Nagro wrote: Thanks! I cannot reproduce the problem on 13-ea b06 (I was using b03). Hi August, I am glad to hear it has fixed your issue. Thank you for confirming that it works with b06! best regards, -- daniel - August

Re: Question about com.sun.net.httpserver.HttpExchange#sendResponseHeaders

, Feb 1, 2019 at 2:58 PM Daniel Fuchs <mailto:daniel.fu...@oracle.com>> wrote: Hi Wembo, On 01/02/19 19:50, Wenbo Zhu wrote: > 1) clarify in the API javadoc that chunked encoding is always applied > even with Connection: close Chunked encoding is always

Re: Question about com.sun.net.httpserver.HttpExchange#sendResponseHeaders

Hi Wenbo, On 06/02/2019 23:21, Wenbo Zhu wrote: Thanks! The text looks good. p.s. is there a way I could get notified of any future updates on this issue? I had thought that I have a java.net account ... If you have the OpenJDK author status you should have the ability to l

Re: RFR [12] 8218546 : Unable to connect to https://google.com using java.net.HttpClient

Hi Chris, The code changes look good to me. They are very limited and look safe and reasonable. The test looks good too - though it's becoming a bit more difficult to read, especially with the mysterious i > 0. I'd suggest adding a comment (or some asserts) in the else branch of the if (lines 2

[13] RFR: 8218554: HttpServer: allow custom handlers to request that the connection be closed after the exchange.

Hi, Please find below a fix for an HttpServer RFE: 8218554: HttpServer: allow custom handlers to request that the connection be closed after the exchange. https://bugs.openjdk.java.net/browse/JDK-8218554 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8218554/webrev.00 The code chan

Re: RFR [12] 8218546 : Unable to connect to https://google.com using java.net.HttpClient

Thanks for that Chris! That's so much better than what I suggested. Reviewed. -- daniel On 07/02/2019 15:08, Chris Hegarty wrote: Thanks for the comments Daniel. I made some simplifications to the test and it now covers the small missing case and is more readable. Updated webrev: http://c

Re: [13] RFR: 8218554: HttpServer: allow custom handlers to request that the connection be closed after the exchange.

Thanks Chris! It's pushed... best regards, -- daniel On 11/02/2019 11:30, Chris Hegarty wrote: Daniel, On 7 Feb 2019, at 13:36, Daniel Fuchs wrote: Hi, Please find below a fix for an HttpServer RFE: 8218554: HttpServer: allow custom handlers to request that the connecti

Updating URI support in the JDK.

Hi, For some time now, I have been conducting an investigation on how to upgrade the JDK to support the newer standards for Generic Uniform Resource Identifiers, namely RFC 3986 [1] and RFC 3987 [2]. I have investigated several possibilities, rejected a few, and prototyped two that looked the mo

Re: RFR 8184315: Typo in java.net.JarURLConnection.getCertificates() method documentation

Looks good Chris. best regards, -- daniel On 11/03/2019 17:03, Chris Hegarty wrote: Trivial typo fixes to: 1) use the simple present tense of the verb, and 2) correctly pluralize the return value. Similar to that of JarEntry::getCertificates. --- a/src/java.base/share/classes/java/net/JarURLC

Re: RFR: 8220083: Use InetAddress.getLoopbackAddress() in place of 127.0.0.1 for some tests

Hi Arthur, I believe this is generally good. The question I have is whether the URLs that use "127.0.0.1" should also be changed to to use whatever address is returned InetAddress.getLoopbackAddress(). If it is needed it may require a bit more coding to ensure that [ ] are correctly added if the

Re: RFR: 8220083: Use InetAddress.getLoopbackAddress() in place of 127.0.0.1 for some tests

Hi Arthur, This looks good to me. I have launched the changes in your second webrev through our test system and not observed any regression. I don't know if Chris still has some tests running, but from my standpoint you're good to go! best regards, -- daniel On 11/03/2019 18:14, Arthur Eubanks

Re: RFR 8220475: Malformed copyright header in LinuxSocketOptions.java, MacOSXSocketOptions.java and MacOSXSocketOptions.c

Looks good Chris! -- daniel On 13/03/2019 12:09, Chris Hegarty wrote: Trivially, there should be a comma after the year. Just add it.

Re: RFR 8220598: Malformed copyright year range in a few files in java.base

Looks good to me Chris! cheers, -- daniel On 13/03/2019 17:04, Chris Hegarty wrote: Trivially, there should be a comma after the year. Just add it.

Re: Regarding 8220575: Correctly format test URI's that contain a retrieved IPv6 address

Hi Chris, On 13/03/2019 17:32, Chris Hegarty wrote: I think that is most cases it should be possible to just replace the use of `getHostAddress` with `getHostName`. This defers the actual lookup to the system configured name service ( rather than trying to encode IPv6 addresses in the test ) I

Re: Regarding 8220575: Correctly format test URI's that contain a retrieved IPv6 address

Hi Arthur, On 13/03/2019 17:43, Arthur Eubanks wrote: Martin suggested looping through all available loopback addresses rather than hardcoding them. In that case we'll need helper test functions to return loopback addresses (and test that it actually returns all available loopback addresses).

Re: RFR 8213912: Semantic typo in HttpExchange.java

Hi Chris, Thanks for fixing this, it is an important distinction! (and I like John's description of the issue :-)) I am wondering though - whether there's a relationship with this bug, and JDK-8180754 [1]. best regards, -- daniel [1] https://bugs.openjdk.java.net/browse/JDK-8180754 On 14/03/

Re: RFR [13] 8170705: sun/net/www/protocol/http/StackTraceTest.java fails intermittently with Invalid Http response

This looks good to me Chris! best regards, -- daniel On 17/03/2019 08:53, Chris Hegarty wrote: This review is for a test only change. It resolves a rare intermittent failure. The issue is that the test creates, retrieves the local port, and immediately closes a server socket. It then expects

Re: RFR [13] 8153508: ContentHandler API contains link to private contentPathProp

Hi Chris, Looks good to me. @value: this is an interesting tag to know! Powerful, useful, maybe dangerous too? In this case the value in question is held in a *private* static final field. So {@link } was clearly wrong. But because the field is private some future maintainer might well be tempt

Re: RFR [13] 8219446: Specify behaviour of timeout accepting methods of Socket and ServerSocket if timeout is negative

Hi Chris, Looks like the right thing to do. +1 -- daniel On 22/03/2019 11:44, Chris Hegarty wrote: This review request is for a specification only change. It clarifies the behaviour of the timeout accepting methods of Socket and ServerSocket, when the given a negative timeout. A negative time

Re: [RFR]: 8220575: Replace hardcoded 127.0.0.1 in URLs with new URI builder

Hi Arthur, I believe this looks good. Here are my comments so far: I like the fact that you kept the builder implementation very minimal, and focused on what these tests actually need. We can always revisit that later if we come across new tests that need more than what your proposed implementat

Re: [RFR]: 8220575: Replace hardcoded 127.0.0.1 in URLs with new URI builder

Hi Arthur, On 26/03/2019 16:51, Arthur Eubanks wrote: Changed B6890349.java to use URL constructor. Changed sequence of URIBuilder calls to `.host().port().path()`. Added missing `.path("/")`. Added logging for most of the constructed URLs. PTAL: http://cr.openjdk.java.net/~aeubanks/8220575/w

RFR: 8221395: HttpClient leaving connections in CLOSE_WAIT state until Java process ends

Hi, Please find below a fix for: 8221395: HttpClient leaving connections in CLOSE_WAIT state until Java process ends https://bugs.openjdk.java.net/browse/JDK-8221395 Webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8221395/webrev.00/ HttpConnection has a high level isOpen() accessor

Re: RFR: JDK-8184770: JDWP support for IPv6

Hi Arthur, Not directly related to Alex's original question but... On 30/03/2019 00:03, Arthur Eubanks wrote: We have some ipv6 patches as well in this area as well (as well as other patches to support ipv6 only environments) that we're trying to upstream. I don't understand the code myself, b

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

Hi Arthur, This looks like a good cleanup to me. On 10/04/2019 21:39, Arthur Eubanks wrote: Here's a prototype webrev to see if the approach is okay with you. If it looks good, I'll continue with the remaining tests I can find. (should I start a new thread for the webrev?) http://cr.openjdk.

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

On 11/04/2019 17:01, Chris Hegarty wrote: Yes, this is a good point. What’s nice about this is that there is just one body of code that provides the functionality ( and it is all in Java, not native). I'm interested to see how this performs in Arthur's experiments, and I need to do a little more

Re: [RFR]: 8222562: IPv6 only systems fail on setsockopt(IPV6_V6ONLY, 0)

Hi Arthur, On 17/04/2019 07:54, Alan Bateman wrote: On 16/04/2019 22:34, Arthur Eubanks wrote: Hi, Copied from the bug https://bugs.openjdk.java.net/browse/JDK-8222562: Some of the networking code tries to support dual socket support. However, it doesn't work with IPv6 only systems. setsock

RFR: 8222527: HttpClient doesn't send HOST header when tunelling HTTP/1.1 through http proxy

Hi, Please find below a fix for: Issue: 8222527: HttpClient doesn't send HOST header when tunnelling HTTP/1.1 through http proxy https://bugs.openjdk.java.net/browse/JDK-8222527 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8222527/webrev.00 When sending the internal CONNECT reque

Re: RFR: 8222527: HttpClient doesn't send HOST header when tunelling HTTP/1.1 through http proxy

regards, -- daniel - Michael. On 17/04/2019, 16:13, Daniel Fuchs wrote: Hi, Please find below a fix for: Issue: 8222527: HttpClient doesn't send HOST header when tunnelling HTTP/1.1 through http proxy https://bugs.openjdk.java.net/browse/JDK-8222527 webrev:

Re: [ipv6] Re: [RFR]: 8222562: IPv6 only systems fail on setsockopt(IPV6_V6ONLY, 0)

Hi Arthur, The jdk.changeset file in your webrev looks completely wrong. http://cr.openjdk.java.net/~aeubanks/ipv6setsockopt/webrev.02/jdk.changeset I assume it's just garbage and we should ignore it. Can you confirm? WRT to the individual files changes listed at http://cr.openjdk.java.net/~aeub

Re: [ipv6] Re: [RFR]: 8222562: IPv6 only systems fail on setsockopt(IPV6_V6ONLY, 0)

Thanks Arthur! I think this is fine. I have done some testing and observed no regression. I believe you should also add the noreg-hard label to 8222562 - since it requires a special set-up to verify that the issue has been fixed. As far as I'm concerned you're good to go with this one! best reg

RFR: 8129315: java/net/Socket/LingerTest.java and java/net/Socket/ShutdownBoth.java timeout intermittently

Hi, Please find below a test stabilization fix for: 8129315: java/net/Socket/LingerTest.java and java/net/Socket/ShutdownBoth.java timeout intermittently http://cr.openjdk.java.net/~dfuchs/webrev_8129315/webrev.00/index.html While testing I stumbled on some other tests failing intermit

Re: RFR: 8129315: java/net/Socket/LingerTest.java and java/net/Socket/ShutdownBoth.java timeout intermittently

Thanks Alan! I have already pushed those changes - but I'll retain this trick for the next batch. FWIW I saw that the default value for the backlog in the impl was 50, so that's what I'd been using. But not having to specify it at all is indeed a better solution. best regards, -- daniel On 27/

Re: RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

Hi Michael, I'm too new to networking libs to actually review this change. However I have eyeballed it and it looks like a very nice simplification and cleanup! I didn't see anything that looked wrong. Two thing though: java/net/Socket.java: (and at multiple other places in this file) 1615

Re: RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

Hi Alan, On 29/04/2019 12:17, Alan Bateman wrote: I don't think AbstractPlainSocketImpl.isBound needs to be volatile as it is guarded by the synchronization on the impl (the doConnect and bind methods are synchronized). I see that it is set outside of any synchronized block in AbstractPlainSo

Re: RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

On 29/04/2019 13:24, Alan Bateman wrote:  433 protected synchronized void bind(InetAddress address, int lport) [...] which for me justifies that it should be volatile. I think you are might be mixing up the lock on the vs. fdLock. From what I can tell, isBound is only accessed by doConnect

RFR: 8223145: [teststabilization] Replace wildcard address with loopback or local host in test - part 1

Hi, Please find below a patch for: 8223145: [teststabilization] Replace wildcard address with loopback or local host in test - part 1 https://bugs.openjdk.java.net/browse/JDK-8223145 http://cr.openjdk.java.net/~dfuchs/webrev_8223145/webrev.00/ This is the first in a series of patches

Re: RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

On 30/04/2019 10:53, Michael McMahon wrote: Thanks for all the comments. A new webrev is at: http://cr.openjdk.java.net/~michaelm/8216978/webrev.2/index.html Looks good Michael! cheers, -- daniel

Re: RFR: 8223145: [teststabilization] Replace wildcard address with loopback or local host in test - part 1

i/java/net/doc-files/net-properties.html -------- *From:* net-dev on behalf of Daniel Fuchs *Sent:* Tuesday 30 April 2019 11:16 *To:* OpenJDK Network Dev list *Subject:* RFR: 8223145: [teststabilization] Replace wildcard address with loopback or local host in test - pa

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

Hi Arthur, On the surface, this looks good. However I have a concern about two things: 1. IPSupport needs to read system properties, attempts to bind sockets etc... I wonder how that might interact with tests that use a security manager, as some of these operations may throw a SecurityE

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

Hi Arthur, On 07/05/2019 00:50, Arthur Eubanks wrote: [...] IIUC, a security manager that disallows everything would prevent the code in IPSupport from working at all, and there's no way to get around it with doPrivileged() since IPSupport and System.getProperty() are not in the same protectio

Re: RFR: 8223457 java.net.ServerSocket protected constructor should throw NPE if impl null

Looks good to me Michael! best regards, -- daniel On 07/05/2019 14:50, Michael McMahon wrote: Could I get the following small change reviewed please? The recent change in the SocketImpl code unmasked the fact that the protected constructor of ServerSocket was not checking for a null impl param

[teststabilization] RFR: 8223463: Replace wildcard address with loopback or local host in tests - part 2

Hi, Some more tests seen failing when using the wildcard address. One of them actually needs the wildcard so for that one I simply edited the summary to make it clear. There are more in the pipe after those are fixed... http://cr.openjdk.java.net/~dfuchs/webrev_8223463/webrev.00/ best regards,

Re: [teststabilization] RFR: 8223463: Replace wildcard address with loopback or local host in tests - part 2

Thaks Alan! On 07/05/2019 16:17, Alan Bateman wrote: This looks good. In passing, it might be clearer if AcceptInheritHandle.test named is argument args rather than sysProps as it's an array of arguments for the child. Also you could use Arrays.stream(sysProps).anyMatch("-Djava.net.preferIPv4S

Re: [ipv6] RFR: 8223532: Don't try creating IPv4 sockets in NetworkInterface.c if IPv4 is not supported

Hi Arthur, The idea looks reasonable to me - I believe the changes in NetworkInterface.c should be OK. However - I don't think the changes in net_util.c::DEF_JNI_OnLoad are acceptable. I'll question whether this is the right place to bail out. I agree it would be nice to have some diagnostic tha

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

Hi Arthur, On 07/05/2019 19:35, Arthur Eubanks wrote: When you said "double checking" I thought it was some terminology related to security managers (e.g. "double checked locking") :P Oh - sorry I was not clear enough. I looked through all the tests I modified for any references to security

Re: [ipv6] Regarding 8220673: Add test library support for determining platform IP support

On 08/05/2019 17:02, Arthur Eubanks wrote: http://cr.openjdk.java.net/~aeubanks/8220673/webrev.04/ LGTM. best regards, -- daniel

[teststabilization] RFR: 8223573: Replace wildcard address with loopback or local host in tests - part 4

Hi, Please find below another patch in the series for intermittently failing tests: 8223573: Replace wildcard address with loopback or local host in tests - part 4 https://bugs.openjdk.java.net/browse/JDK-8223573 patch: http://cr.openjdk.java.net/~dfuchs/webrev_8223573/webrev.00/ best

Re: [teststabilization] RFR: 8223573: Replace wildcard address with loopback or local host in tests - part 4

On 09/05/2019 08:09, Alan Bateman wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8223573/webrev.00/ I skimmed through the diffs and they look okay with me. Minor nit in 6550798/test.java is there is a space in "HttpServer.create (". Also wonder if we should rename this source file too. Than

Re: [teststabilization] RFR: 8223573: Replace wildcard address with loopback or local host in tests - part 4

Hi Chris, On 09/05/2019 10:33, Chris Hegarty wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8223573/webrev.01/ Looks good. I’m missing why ChunkedEncodingWithProgressMonitorTest.java needs `@library /test/lib` added? It depends on ChunkedInputStream/ChunkedEncodingTest.java, creates an ins

Re: [teststabilization] RFR: 8223573: Replace wildcard address with loopback or local host in tests - part 4

On 09/05/2019 10:52, Alan Bateman wrote: Thanks, I see there is no more at L48 (you've removed one of them but there is a second). In any case, the updated webrev looks good. Argh! Thanks Alan - well spotted. I'll remove the offending space before pushing. best regards, -- daniel

Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

Hi Christoph, I'm only commenting on the HttpClient changes, I'll let others comment on the other changes... http://cr.openjdk.java.net/~clanger/webrevs/8223553.0/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java.udiff.html I have a profound dislike for using @SuppressedWa

Re: [RFR]: 8223652: Rename IPSupport.skipIfCurrentConfigurationIsInvalid()

Hi Arthur, Looks good to me. best regards, -- daniel On 09/05/2019 18:40, Arthur Eubanks wrote: Bug: https://bugs.openjdk.java.net/browse/JDK-8223652 Webrev: http://cr.openjdk.java.net/~aeubanks/8223652/webrev.00/index.html As mentioned in https://markmail.org/search/?q=aeubanks%40google.co

Re: [teststabilization] RFR: 8223465: Replace wildcard address with loopback or local host in tests - part 3

Hi Aleksei, ServerSocket_accept.java: I think the new logic deserve a comment before line 63. Something like: // if readyToClose is still false it means some other // process on the system attempted to connect: just // ignore it, and go back to accept again. SocksV4Test.java: can you remove t

Re: [teststabilization] RFR: 8223465: Replace wildcard address with loopback or local host in tests - part 3

Looks god to me! cheers, -- daniel On 10/05/2019 13:24, Aleks Efimov wrote: Thank you for the review Daniel! I've added the suggested comment to ServerSocket_accept + fixed the white-spaces. About ChunkedErrorStream.java: Thank you for spotting the change - I will keep the original logic

Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

Hi Christoph, On 13/05/2019 08:29, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should have double checked. Can we at least reduce the scope of the @SuppressedWarnings by introducing a private method t

[teststabilization] RFR: 8223632: Replace wildcard address with loopback or local host in tests - part 5

Hi, Please find below a fix for a bunch of tests that have been observed to fail intermittently. The usual trick (replacing wildcard with localhost) was applied. In some cases, the logic of the test meant that the trick could be applied only partially (e.g. it could be applied to the proxy, but

Re: [teststabilization] RFR: 8223638: Replace wildcard address with loopback or local host in tests - part 6

Hi Aleksei, I believe that some configurations in the wild might return you the external host address when looking up "localhost". It doesn't matter if the server binds to the wildcard, but if you change the server to stop using the wildcard, then you also need to change the client to use the sam

RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic

Hi, Please find below a fix for: [1] 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic Occasionally a test server may receive traffic that doesn't originate from the client in the test. If the client makes requests that are recogn

Re: [teststabilization] RFR: 8223638: Replace wildcard address with loopback or local host in tests - part 6

us, I've utilized URIBuilder where possible. Also tried keep the SimpleHttpServer simpler. The new version can be viewed here: http://cr.openjdk.java.net/~aefimov/8223638/01/ With Best Regards, Aleksei On 13/05/2019 16:45, Daniel Fuchs wrote: Hi Aleksei, I believe that some configurations i

Re: [teststabilization] RFR: 8223638: Replace wildcard address with loopback or local host in tests - part 6

Hi Aleksei, Looks good to me! best regards, -- daniel On 14/05/2019 12:22, Aleks Efimov wrote: Hi Daniel, New webrev can be found here: http://cr.openjdk.java.net/~aefimov/8223638/02/ PerConnectionProxy.java - fixed as suggested. MultiReleaseJarURLConnection.java: I've added the toHttpJa

RFR: 8223880: Update sun/net/ftp/FtpURL.java and sun/net/ftp/FtpURLConnectionLeak.java to work with IPv6 addresses

Hi, Some recent test stabilization fixes had a side effect on two FTP tests: now that the client might use the IPv6 loopback, the test might fail on IPv6 only hosts if the test server doesn't support the extended EPSV mode, or if the client isn't configured to use NO_PROXY and the default proxy s

Re: RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

Hi Christoph, That looks much better, thanks! (but still not commenting on the other changes ;-)) best regards, -- daniel On 14/05/2019 13:57, Langer, Christoph wrote: Hi Daniel, unfortunately, your proposed solution does not work with javac. I get this in the build: Oh darn. I should hav

Re: RFR: 8223880: Update sun/net/ftp/FtpURL.java and sun/net/ftp/FtpURLConnectionLeak.java to work with IPv6 addresses

Hi Arthue, On 14/05/2019 23:57, Arthur Eubanks wrote: In test/jdk/sun/net/ftp/FtpURL.java, extendedEnabled is always true. Same with portEnabled and pasvEnabled. Yes - I left them there as they were preexisting. The bits that handles EPSV is a copy paste from another test and that minimized t

Re: RFR: 8223880: Update sun/net/ftp/FtpURL.java and sun/net/ftp/FtpURLConnectionLeak.java to work with IPv6 addresses

Hi Vyom, On 15/05/2019 11:25, Vyom Tiwari wrote: Hi Daniel, Changes looks good to me, Thanks! as you said code is copied from one test to another, i found FtpGetContent.java where same FtpServer code is copied.  Are you planning to fix FtpGetContent.java as well  ?. Well - I wasn't inten

Re: [ipv6] RFR: 8223532: Don't try creating IPv4 sockets in NetworkInterface.c if IPv4 is not supported

On 15/05/2019 16:19, Arthur Eubanks wrote: New webrev: http://cr.openjdk.java.net/~aeubanks/8223532/webrev.03/ All good. I see no issues when running various tests. -Chris. Thanks, still need one more reviewer. Hi Arthur, This is not hotspot - I don't think you do :-) But FWIW

Re: [ipv6] RFR: 8223532: Don't try creating IPv4 sockets in NetworkInterface.c if IPv4 is not supported

Hi Arthur, On 15/05/2019 17:11, Arthur Eubanks wrote: I thought I always needed two people to say the change looks good. Is only needing one reviewer a JDK policy or a net-dev policy? It depends on the project/group rules. AFAIK you only need two reviewers when you contribute changes to hotspo

Re: [teststabilization] RFR: 8223798 : Replace wildcard address with loopback or local host in tests - part 7

Hi Aleksei, On 15/05/2019 17:07, Aleks Efimov wrote: Hi, Another part of test fixes to address intermittent networking test failures can be viewed here: http://cr.openjdk.java.net/~aefimov/8223798/00/ Socket_getInputStream_read.java: Socket_getOutputStream_write.java: I think you could sim

Re: [teststabilization] RFR: 8223798 : Replace wildcard address with loopback or local host in tests - part 7

On 15/05/2019 18:41, Aleks Efimov wrote: Hi Daniel, Thanks for the review. I've modified Socket_getInputStream_[read|write] to follow your suggestion: -    ServerSocket ss = new ServerSocket(0); InetAddress lh = InetAddress.getLocalHost(); +    ServerSocket ss = n

Re: RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic

On 15/05/2019 16:17, Chris Hegarty wrote: I believe workers should be volatile/synchronized, as it is written and read from different threads, no? Thanks Chris. workers is a concurrent queue, but the variable should be final. I'll do it before pushing. best regards, -- daniel -Chris.

Re: RFR: 8223716: sun/net/www/http/HttpClient/MultiThreadTest.java should be more resilient to unexpected traffic

Hi Mark, AFAIU the test wants to verify that connections are reused. It does that by creating N (N=5) threads that will hammer at the server concurrently. Because there are N threads, then there can't be more than N concurrent requests, and therefore there should not be more than N connections a

Re: RFR: JDK-8224028: loop initial declarations introduced by JDK-8184770 (jdwp)

Hi Ao Qi, I'm adding serviceability-dev, since this is for jdwp. The proposed changes look good to me - but please get someone from the serviceability team to review this. best regards, -- daniel On 16/05/2019 08:41, Ao Qi wrote: Hi, I found build is failed on CentOS 7.6, because of loop i

Re: [ipv6]: 8224014: Don't run test/jdk/java/net/NetworkInterface/IPv4Only.java in IPv6 only environment

Hi Arthur, Looks good to me. Reviewed. best regards, -- daniel On 16/05/2019 00:24, Arthur Eubanks wrote: webrev: http://cr.openjdk.java.net/~aeubanks/8224014/webrev.00/ bug: https://bugs.openjdk.java.net/browse/JDK-8224014 In https://bugs.openjdk.java.net/browse/JDK-8220673, I missed skippi

Re: [ipv6]: 8224019: test/jdk/java/nio/channels/DatagramChannel/BasicMulticastTests.java assumes IPv4 is always available

Hi Arthur, Shouldn't there be a version of 221 exceptionTests(nif); that works with IPv6? If no machine has IPv4 available, then these exceptions won't be tested any more. I suspect the test made the assumption that IPv4 was always available, and therefore testing exceptions with I

Re: [ipv6]: 8224018: test/jdk/java/net/ipv6tests/{Tcp,Udp}Test.java assume IPv4 is available

Hi Arthur, Looks good to me! best regards, -- daniel On 16/05/2019 01:46, Arthur Eubanks wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8224018 webrev: http://cr.openjdk.java.net/~aeubanks/8224018/webrev.00/ test/jdk/java/net/ipv6tests/{Tcp,Udp}Test.java both assume that IPv4 is avail

[teststabilization] RFR: 8223856: Replace wildcard address with loopback or local host in tests - part 8

Hi, Please find below a fix for [1]: 8223856: Replace wildcard address with loopback or local host in tests - part 8 http://cr.openjdk.java.net/~dfuchs/webrev_8223856/webrev.00/index.html Some of the test failures weren't all attributable to the use of the wildcard: in some cases tests

Re: [ipv6]: 8224014: Don't run test/jdk/java/net/NetworkInterface/IPv4Only.java in IPv6 only environment

Hi Arthur, On 16/05/2019 18:01, Arthur Eubanks wrote: Good idea, done. PTAL. Tested with and without IPv4 support. http://cr.openjdk.java.net/~aeubanks/8224014/webrev.01/index.html Shouldn't you remove: 38 IPSupport.throwSkippedExceptionIfNonOperational(); In this case? best regar

Re: [ipv6]: 8224081: SOCKS v4 doesn't work with IPv6

Hi Arthur, On 17/05/2019 00:16, Arthur Eubanks wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8224081 webrev: http://cr.openjdk.java.net/~aeubanks/8224081/webrev.00/index.html Tests that try to use SOCKS v4 will fail in an IPv6 only environment since SOCKS v4 does not support IPv6. SOCKS

Re: [ipv6] RFR: 8223214: Inet6AddressImpl.loopbackAddress() should choose loopback address that is available

Hi Arthur, On 17/05/2019 00:18, Arthur Eubanks wrote: I've tried to run this through the submit repo twice, haven't gotten a result from either run. Could somebody check what's happened to those runs? The branch names are "JDK-8223214" and "JDK-8223214-1". I'm not seeing anything for JDK-8223

Re: [ipv6]: 8224014: Don't run test/jdk/java/net/NetworkInterface/IPv4Only.java in IPv6 only environment

On 17/05/19 18:02, Arthur Eubanks wrote: Oh yes, http://cr.openjdk.java.net/~aeubanks/8224014/webrev.02/ Made sure that the "caught expected exception" lines were printed out under IPv6-only environment. Yes, looks good! best regards, -- daniel

Re: [ipv6]: 8224019: test/jdk/java/nio/channels/DatagramChannel/BasicMulticastTests.java assumes IPv4 is always available

On 17/05/2019 18:22, Arthur Eubanks wrote: Done, order is protocol family, group/notGroup InetAddress, NetworkInterface, source InetAddress http://cr.openjdk.java.net/~aeubanks/8224019/webrev.02/ Looks good to me as well Arthur! best regard, -- daniel

<    1   2   3   4   5   6   7   8   9   10   >