RFR (11): 8206001 Enable TLS1.3 by default in Http Client

2018-07-04 Thread Michael McMahon
Hi, Can I get the following change reviewed please for 11? It is to enable the use of TLS1.3 in the http client by default plus associated changes caused by the new implementation. http://cr.openjdk.java.net/~michaelm/8206001/webrev.01/ Thanks, Michael.

Re: RFR [11] 8207265: Bad HTML in {@link} in HttpResponse.BodySubscribers.ofPublisher

2018-07-16 Thread Michael McMahon
Looks fine Chris. - Michael. On 16/07/2018, 12:10, Chris Hegarty wrote: This is a review request for a small doc-only change to fix a "bad" use of angle brackets ( for a parameterized type ) as the target of an @link [*]. The simplest solution is to just replace the link with @code, since the b

Re: Bug in HttpClient

2018-07-20 Thread Michael McMahon
Thanks for reporting this. I will look into it. - Michael On 20/07/2018, 08:38, Severin Gehwolf wrote: Adding net-dev On Fri, 2018-07-20 at 08:52 +0200, Thomas Lußnig wrote: Hi, i found an bug in JDK 10 with the new HttpClient. It does not handle responses wihtout contentlength correctly. Nor

Re: RFR [11 8207959 The initial value of SETTINGS_MAX_CONCURRENT_STREAMS should have no limit

2018-07-24 Thread Michael McMahon
This looks fine, Chris. - Michael. On 23/07/2018, 11:43, Chris Hegarty wrote: The initial value for the directional parameter SETTINGS_MAX_CONCURRENT_STREAMS, in the direction from the server to the client, max concurrent client-initiated streams, is incorrectly limited to 100, when there shoul

Re: RFR [11] 8207960: Non-negative WINDOW_UPDATE increments may leave the stream window size negative

2018-07-24 Thread Michael McMahon
Looks good Chris - Michael. On 24/07/2018, 10:23, Chris Hegarty wrote: The stream window size can correctly become negative after processing the initial SETTINGS_INITIAL_WINDOW_SIZE. Stream specific WINDOW_UPDATE's should not cause a stream reset if the current window size is negative before th

Re: Connection timeouts in JEP 321 (HTTP Client)

2018-07-25 Thread Michael McMahon
Hi Markus, The API and implementation supports an overall response timeout through the method: HttpRequest.Builder timeout​(Duration duration) So, I don't think any application should be required to wait for 130 seconds. It does not currently have a timeout setting specific for connection s

Re: Connection timeouts in JEP 321 (HTTP Client)

2018-08-01 Thread Michael McMahon
Simone, Just to clarify a couple of points below: On 31/07/2018, 22:07, Simone Bordet wrote: Hi, On Tue, Jul 31, 2018 at 9:58 PM Chris Hegarty wrote: It is unfortunate that this has come up so late the JDK 11 lifecycle, but since this has API impact, and is the right thing to do, it is cert

Re: Connection timeouts in JEP 321 (HTTP Client)

2018-08-02 Thread Michael McMahon
On 31/07/2018, 22:07, Simone Bordet wrote: .. Finally, I would consider TLS handshake time as part of the request data: the cost will be paid by the first request and capped by "total" timeout (not connectTimeout). With TLS 1.3 optimizations, it may be just few TLS bytes before the encrypted

Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API

2018-08-08 Thread Michael McMahon
The updated webrev looks fine to me, Chris. Thanks, Michael On 07/08/2018, 10:52, Chris Hegarty wrote: On 4 Aug 2018, at 14:08, Jaikiran Pai wrote: ... Do you think this can be reworded a bit? Although I understand what's being said here, the wording doesn't seem right. Maybe something like:

Re: Suggestion for the HTTP client: allow selection of a local address

2018-08-08 Thread Michael McMahon
Klaus I've created an RFE at https://bugs.openjdk.java.net/browse/JDK-8209137 to track this. I imagine the simplest solution would be to be able to specify the local address when constructing a HttpClient, and then all connections created using that client would be bound to the chosen address.

RFR [11]: 8207966 HttpClient response without content-length does not return body

2018-08-14 Thread Michael McMahon
Hi, This is an important fix for 11 which addresses the problem where HTTP/1.0 responses that do not include a content-length header are not handled correctly. http://cr.openjdk.java.net/~michaelm/8207966/webrev.2/index.html Thanks, Michael.

Re: RFR [11]: 8207966 HttpClient response without content-length does not return body

2018-08-15 Thread Michael McMahon
. Gruß Thomas Lußnig p.s. Great to see that an fix is available soon. On 14.08.2018 15:59:51, Michael McMahon wrote: Hi, This is an important fix for 11 which addresses the problem where HTTP/1.0 responses that do not include a content-length header are not handled correctly. http

RFR: 8210311: IllegalArgumentException in CookieManager - Comparison method violates its general contract

2018-09-12 Thread Michael McMahon
Could I get the following reviewed please? It fixes a problem in j.n.CookieManager where an internal Comparator was not obeying its contract and crashing occasionally in Collections.sort(). It also updates the behavior (sorting of cookie headers) to RFC6265. http://cr.openjdk.java.net/~michael

Re: RFR: 8210311: IllegalArgumentException in CookieManager - Comparison method violates its general contract

2018-09-13 Thread Michael McMahon
17:28, Michael McMahon wrote: Could I get the following reviewed please? It fixes a problem in j.n.CookieManager where an internal Comparator was not obeying its contract and crashing occasionally in Collections.sort(). It also updates the behavior (sorting of cookie headers) to RFC6265. http

RFR: 8211420: com.sun.net.httpserver.HttpServer returns Content-length header for 204 response code

2018-10-04 Thread Michael McMahon
Could I get the following fix reviewed please? http://cr.openjdk.java.net/~michaelm/8211420/webrev.1/index.html Thanks, Michael

Re: RFR: 8211420: com.sun.net.httpserver.HttpServer returns Content-length header for 204 response code

2018-10-04 Thread Michael McMahon
On 04/10/2018 17:09, Michael McMahon wrote: Could I get the following fix reviewed please? http://cr.openjdk.java.net/~michaelm/8211420/webrev.1/index.html Thanks, Michael

Re: HttpClient Headers

2018-10-05 Thread Michael McMahon
restricted? Because i have scenarios where the customer server expect Referer header in the Login sequence. So is there any way how to set restricted headers? Gruß Thomas On 14.08.2018 15:59:51, Michael McMahon wrote: Hi, This is an important fix for 11 which addresses the problem where

RFR [12]: 8203850: java.net.http HTTP client should allow specifying Origin and Referer headers

2018-10-11 Thread Michael McMahon
Could I get the following fix reviewed please? http://cr.openjdk.java.net/~michaelm/8203850/webrev.1/ Thanks, Michael

Re: RFR [12]: 8203850: java.net.http HTTP client should allow specifying Origin and Referer headers

2018-10-11 Thread Michael McMahon
test/jdk/java/net/SpecialHeadersTest.java too. best regards, -- daniel On 11/10/2018 13:28, Michael McMahon wrote: Could I get the following fix reviewed please? http://cr.openjdk.java.net/~michaelm/8203850/webrev.1/ Thanks, Michael

Re: RFR [12]: 8203850: java.net.http HTTP client should allow specifying Origin and Referer headers

2018-10-11 Thread Michael McMahon
if (response.statusCode() != 200) 220 throw new RuntimeException("illegal header was sent in request"); Shouldn't this always throw, regardless of the status code? best regards, -- daniel On 11/10/2018 16:56, Michael McMahon wrote: Thanks Daniel. I up

RFR [12]: java.net.http.HttpClient hangs on 204 reply without Content-length 0

2018-10-15 Thread Michael McMahon
Could I get the following fix reviewed please. http://cr.openjdk.java.net/~michaelm/8211437/webrev.1/index.html Thanks, Michael.

Re: RFR [12]: java.net.http.HttpClient hangs on 204 reply without Content-length 0

2018-10-15 Thread Michael McMahon
Hi Daniel, On 15/10/2018, 12:53, Daniel Fuchs wrote: Hi Michael, On 15/10/2018 11:54, Michael McMahon wrote: Could I get the following fix reviewed please. http://cr.openjdk.java.net/~michaelm/8211437/webrev.1/index.html Looks good in general. In MultiExchange.java: 236 T

Re: RFR [12]: 8203850: java.net.http HTTP client should allow specifying Origin and Referer headers

2018-10-15 Thread Michael McMahon
nd Referer to test/jdk/java/net/SpecialHeadersTest.java too. best regards, -- daniel On 11/10/2018 13:28, Michael McMahon wrote: Could I get the following fix reviewed please? http://cr.openjdk.java.net/~michaelm/8203850/webrev.1/ Thanks, Michael

Re: RFR [12] 8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0

2018-10-18 Thread Michael McMahon
user explicitly sets one, then it will be sent. This is useful for testing this fix here. Thanks, Michael On 15/10/2018, 16:07, Michael McMahon wrote: Hi Daniel, On 15/10/2018, 12:53, Daniel Fuchs wrote: Hi Michael, On 15/10/2018 11:54, Michael McMahon wrote: Could I get the following fix

Re: RFR [12] 8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0

2018-10-18 Thread Michael McMahon
, HTTPS/2 ? best regards, -- daniel On 18/10/2018 14:58, Michael McMahon wrote: Updated webrev for this at http://cr.openjdk.java.net/~michaelm/8211437/webrev.2/index.html based on feedback below. I also made a change to the com.sun httpserver. It changes the recent fix related to the same issue such t

Re: RFR [12] 8212695: Add explicit timeout to several HTTP Client tests

2018-10-19 Thread Michael McMahon
Looks fine Chris. - Michael On 19/10/2018, 17:12, Chris Hegarty wrote: There has been a recent change to Oracle's internal test environment that now causes a couple of HTTP Client tests to timeout before they have had a chance to complete their execution, resulting in what ( incorrectly ) appea

RFR: 8212926 HttpClient does not retrieve files with large sizes over HTTP/1.1

2018-10-25 Thread Michael McMahon
Hi, Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/8212926/webrev.1/index.html We would like to backport it to jdk11u also. Thanks, Michael

Re: Date header and Java 11 HTTP client

2018-10-31 Thread Michael McMahon
We have filed https://bugs.openjdk.java.net/browse/JDK-8213189 "Make restricted headers in HTTP Client configurable and remove Date by default" which should deal with this finally. - Michael On 31/10/2018, 17:46, Thomas Lußnig wrote: Hi all, from the count of problems that are upcomming

Re: Option to supply custom hostname verifier to HTTP client

2018-11-01 Thread Michael McMahon
You could also isolate the behavior to a specific SSLContext (and therefore HttpClient) by initializing the SSLContext with a dummy TrustManager (if it's only for testing). - Michael. On 01/11/2018, 18:09, Anders Wisch wrote: Thankfully, all of my uses are for testing. To test hostname-based

Re: Option to supply custom hostname verifier to HTTP client

2018-11-02 Thread Michael McMahon
the URI's host, and the host header was customizable, then a self-signed certificate with CN and SAN entries for all of the names in question would work (provided I supplied a dummy TrustManager like you suggested). On Nov 1, 2018, at 11:45 AM, Michael McMahon wrote: You could also

Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Michael McMahon
Hi, This is the right place to ask! It was implemented that way because RFC 7540 recommends that: Clients SHOULD NOT open more than one HTTP/2 connection to a given host and port pair, where the host is derived from a URI, a selected alternative service [ALT-SVC

Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Michael McMahon
That's useful feedback Simone. It sounds like this is a somewhat different issue though. How would you recommend determining when to open a new connection to maximise throughput? Thanks, Michael. On 07/11/2018, 12:00, Simone Bordet wrote: Hi, On Wed, Nov 7, 2018 at 11:44 AM Michael Mc

RFR 8213189: Make restricted headers in HTTP Client configurable and remove Date by default

2018-11-09 Thread Michael McMahon
Could I get the following change reviewed please? We've reduced the number of restricted headers to the minimum and made those settable via a networking property (with health warning) http://cr.openjdk.java.net/~michaelm/8213189/webrev.1/ This change will require a CSR for the new property. Th

RFR 8213616 URLPermission with query or fragment behaves incorrectly

2018-11-09 Thread Michael McMahon
Could i get the following small change reviewed please? http://cr.openjdk.java.net/~michaelm/8213616/webrev.1/ Thanks, Michael

Re: RFR 8213616 URLPermission with query or fragment behaves incorrectly

2018-11-12 Thread Michael McMahon
On 12/11/2018, 11:14, Chris Hegarty wrote: Michael, On 09/11/18 15:48, Michael McMahon wrote: Could i get the following small change reviewed please? http://cr.openjdk.java.net/~michaelm/8213616/webrev.1/ The change looks ok to me. Just a few comments: 1) The specification already mentions

RFR: 8211842 IPv6_supported wrongly returns false when unix domain socket is bound to fd 0

2018-11-28 Thread Michael McMahon
Could I get the following fix reviewed please? The fix is pretty simple, but the test a bit more involved since the issue occurs when the JVM is launched in unusual circumstances where its stdin,out,err are connected to a UNIX domain socket. This occurs when java is launched by a node.js or Python

Re: RFR: 8211842 IPv6_supported wrongly returns false when unix domain socket is bound to fd 0

2018-11-29 Thread Michael McMahon
On 29/11/2018, 17:19, Alan Bateman wrote: On 28/11/2018 15:50, Michael McMahon wrote: Could I get the following fix reviewed please? The fix is pretty simple, but the test a bit more involved since the issue occurs when the JVM is launched in unusual circumstances where its stdin,out,err are

Re: 8211842 IPv6_supported wrongly returns false when unix domainsocket is bound to fd 0

2018-11-29 Thread Michael McMahon
Yes, as Alan said the way IPv6 is set up in the JDK means that either you have: AF_INET sockets (which are IPv4 only) or AF_INET6 sockets (which can be IPv4 or IPv6) It's not possible to mix the two types. If inetd passes an AF_INET socket on startup then the VM is stuck in IPv4 mode. I've upd

RFR: 8199849 HttpServer/BasicAuthenticator: unicode bytes are not correctly handled and no workaround is provided

2018-12-12 Thread Michael McMahon
Could I get the following webrev reviewed please? http://cr.openjdk.java.net/~michaelm/8199849/webrev.1/ A CSR for the small API change will follow. Thanks, Michael

Re: RFR: 8199849 HttpServer/BasicAuthenticator: unicode bytes are not correctly handled and no workaround is provided

2018-12-12 Thread Michael McMahon
good idea. Thanks, Michael best regards, -- daniel On 12/12/2018 10:51, Michael McMahon wrote: Could I get the following webrev reviewed please? http://cr.openjdk.java.net/~michaelm/8199849/webrev.1/ A CSR for the small API change will follow. Thanks, Michael

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

2019-01-16 Thread Michael McMahon
Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/8217237/webrev.1/ Thanks, Michael

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

2019-01-17 Thread Michael McMahon
p; best regards, -- daniel On 16/01/2019 16:46, Michael McMahon wrote: Could I get the following change reviewed please? http://cr.openjdk.java.net/~michaelm/8217237/webrev.1/ Thanks, Michael

Re: 8217451: ExtendedSocketOptions should encapsulate support for SO_FLOW_SLA

2019-01-21 Thread Michael McMahon
Hi Alan, Looks fine apart from extra space on line 85: ExtendedSocketOptions.java - Michael. On 21/01/2019, 17:41, Alan Bateman wrote: ExtendSocketOptions is the supporting class that the socket implementations use for JDK-specific socket options. Vyom improved it last year to select options

Re: 8217461: (ch) Add Net.available to return the number of bytes in the socket input buffer

2019-01-22 Thread Michael McMahon
Looks fine to me Alan. - Michael. On 21/01/2019, 19:10, Alan Bateman wrote: This is a small change to add a method to sun.nio.ch.Net to get the number of bytes in the socket input buffer. The motive for adding this to make it possible for the socket adaptors to implement InputStream::availabl

RFR: 8216986 Remove unused code from SocksSocketImpl

2019-01-23 Thread Michael McMahon
Hi Could I get the following webrev reviewed please? It is just to remove dead code from SocksSocketImpl. Most of the code was an (unused) attempt to implement SOCKS for ServerSockets. getLocalPort() was potentially buggy and should not override the super class implementation. So, that was remove

Re: RFR: 8216986 Remove unused code from SocksSocketImpl

2019-01-23 Thread Michael McMahon
On 23/01/2019, 12:37, Alan Bateman wrote: On 23/01/2019 11:44, Michael McMahon wrote: Hi Could I get the following webrev reviewed please? It is just to remove dead code from SocksSocketImpl. Most of the code was an (unused) attempt to implement SOCKS for ServerSockets. getLocalPort() was

Re: RFR: 8216986 Remove unused code from SocksSocketImpl

2019-01-24 Thread Michael McMahon
14:30, Michael McMahon wrote: On 23/01/2019, 12:37, Alan Bateman wrote: On 23/01/2019 11:44, Michael McMahon wrote: Hi Could I get the following webrev reviewed please? It is just to remove dead code from SocksSocketImpl. Most of the code was an (unused) attempt to implement SOCKS for ServerSo

Re: RFR: 8216986 Remove unused code from SocksSocketImpl

2019-01-24 Thread Michael McMahon
On 24/01/2019, 14:38, Alan Bateman wrote: On 24/01/2019 13:09, Michael McMahon wrote: I've updated the webrev at http://cr.openjdk.java.net/~michaelm/8216986/webrev.2/ to add some tests and also I found the same dubious implementation of getLocalPort() in HttpConnectSocketImpl.java

Re: RFR [12] 8218662: Allow 204 responses with Content-Length:0

2019-02-08 Thread Michael McMahon
Looks good Chris. - Michael. On 08/02/2019, 11:24, Chris Hegarty wrote: This is a code review request for a late fix for JDK 12, to address a what could be perceived as a potential regression. Post 8211437 [1] the HTTP Client now disallows 204 responses with a response body. It appears that th

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

2019-03-11 Thread Michael McMahon
This looks good to me. - Michael. On 11/03/2019, 16:55, Arthur Eubanks wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8220083 webrev: http://cr.openjdk.java.net/~aeubanks/8220083/webrev.00/ First of a few ipv6 patches to come.

Re: RFR [13] 8220663: Incorrect handling of IPv6 addresses in Socket(Proxy.HTTP)

2019-03-19 Thread Michael McMahon
This looks good to me too. - Michael. On 17/03/2019, 09:14, Chris Hegarty wrote: This review request resolves an issue where IPv6 socket addresses, used in socket connect, are not correctly enclosed in square brackets when tunneling over HTTP. http://cr.openjdk.java.net/~chegar/8220663/ -Chri

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

2019-03-27 Thread Michael McMahon
Fix looks good to me Daniel. Good catch. - Michael. On 27/03/2019, 17:02, Daniel Fuchs wrote: 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.openj

Re: JDK-8170910: Avoid 5 second localhost lookup behavior on OSX

2019-04-04 Thread Michael McMahon
Hi Nora, At first sight, the approach sounds reasonable to me. I'd like to see the patch and also do you have detailed instructions on how to reproduce the issue (the 5 second delay)? Thanks, Michael. On 03/04/2019, 17:46, Nora Howard wrote: I'd initially sent this to jdk-dev, but I was ask

Re: JDK-8170910: Avoid 5 second localhost lookup behavior on OSX

2019-04-05 Thread Michael McMahon
nhoward.local/10.0.0.3 <http://10.0.0.3> 5044 nhoward.local/10.0.0.3 <http://10.0.0.3> 0 nhoward.local/10.0.0.3 <http://10.0.0.3> 0 nhoward.local/10.0.0.3 <http://10.0.0.3> 0 On JDK 8, each request takes 5 seconds. JDK 8's address lookup cache works differently. It

Re: JDK-8170910: Avoid 5 second localhost lookup behavior on OSX

2019-04-09 Thread Michael McMahon
t A records reach: 0x (Not Reachable) order: 30 On Fri, Apr 5, 2019 at 10:14 AM Michael McMahon mailto:michael.x.mcma...@oracle.com>> wrote: I've spent some time trying to reproduce this on 10.13 but have not been able. I have disabled all the s

Re: ServerSocket.isBound() continue to return true after close() is called.

2019-04-11 Thread Michael McMahon
Norman The specification on what happens to all socket types was updated many years ago in bug id 6505016, but it looks like ServerSocket::isBound was missed from that effort. I think we should probably update the spec to reflect current behavior and be consistent with the change above. There

Re: AW: JDK-8170910: Avoid 5 second localhost lookup behavior on OSX

2019-04-11 Thread Michael McMahon
onicalHostName() method which does do a reverse lookup (via getnameinfo()) Michael *Von: *Michael McMahon <mailto:michael.x.mcma...@oracle.com> *Gesendet: *Dienstag, 9. April 2019 18:35 *An: *Nora Howard <mailto:nhow...@twitter.com> *Cc: *net-dev@openjdk.java.net <mailto:net-dev

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

2019-04-17 Thread Michael McMahon
Hi Daniel, Looks fine. Though is the HOST_ALLOWED predicate actually needed in Utils.java? Can you not do the negation inline directly in the HOST_RESTRICTED predicate? - Michael. On 17/04/2019, 16:13, Daniel Fuchs wrote: Hi, Please find below a fix for: Issue: 8222527: HttpClient doesn't

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

2019-04-18 Thread Michael McMahon
Thanks Daniel. All good. - Michael. On 17/04/2019, 18:00, Daniel Fuchs wrote: Hi Michael, On 17/04/2019 17:06, Michael McMahon wrote: Looks fine. Though is the HOST_ALLOWED predicate actually needed in Utils.java? Can you not do the negation inline directly in the HOST_RESTRICTED predicate

RFR 8216978: Drop support for pre JDK 1.4 SocketImpl implementations

2019-04-29 Thread Michael McMahon
Hi, This is another change which is part of the general cleanup of SocketImpls. The change removes support for pre JDK 1.4 socketimpls which do not implement the timed connect method. These SocketImpls have not been compilable since 1.4 and limited runtime support has been provided since then,

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

2019-04-29 Thread Michael McMahon
Thanks Chris. Comments noted. - Michael On 29/04/2019, 11:26, Chris Hegarty wrote: Michael, On 29/04/2019 10:52, Michael McMahon wrote: Hi, This is another change which is part of the general cleanup of SocketImpls. The change removes support for pre JDK 1.4 socketimpls which do not

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

2019-04-29 Thread Michael McMahon
Hi Daniel, On 29/04/2019, 11:34, Daniel Fuchs wrote: 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

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

2019-04-29 Thread Michael McMahon
On 29/04/2019, 13:08, Chris Hegarty wrote: On 29/04/2019 12:17, Alan Bateman wrote: ... Changing SIS.close and SOS.close to caller super.close raises a number of questions. These close should never be called Socket.getInputStream and getOutputStream don't leak these streams to user code (

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

2019-04-29 Thread Michael McMahon
On 29/04/2019, 17:35, David Lloyd wrote: On Mon, Apr 29, 2019 at 10:54 AM Michael McMahon wrote: On 29/04/2019, 13:08, Chris Hegarty wrote: On 29/04/2019 12:17, Alan Bateman wrote: ... Changing SIS.close and SOS.close to caller super.close raises a number of questions. These close

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

2019-04-29 Thread Michael McMahon
On 29/04/2019, 12:17, Alan Bateman wrote: On 29/04/2019 10:52, Michael McMahon wrote: Hi, This is another change which is part of the general cleanup of SocketImpls. The change removes support for pre JDK 1.4 socketimpls which do not implement the timed connect method. These SocketImpls

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

2019-04-30 Thread Michael McMahon
Thanks for all the comments. A new webrev is at: http://cr.openjdk.java.net/~michaelm/8216978/webrev.2/index.html The CSR now also includes the minor API doc update suggested for the no-arg SocketImpl constructor. Thanks, Michael On 29/04/2019, 10:52, Michael McMahon wrote: Hi, This is

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

2019-05-07 Thread Michael McMahon
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 parameter as required. http://cr.openjdk.java.net/~michaelm/8223457/webrev.1/ Thanks Michael

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

2019-05-07 Thread Michael McMahon
Thanks. On 07/05/2019, 14:55, Daniel Fuchs wrote: 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

Re: 8218559: Reimplement the Legacy Socket API

2019-05-07 Thread Michael McMahon
Hi Alan, What's the purpose of the change to the UdpTest? - Michael. On 01/05/2019, 14:10, Alan Bateman wrote: JEP 353 [1] is now a candidate and I would like to get the CSR [2] finalized and the changes reviewed so that it can be targeted. The webrev with the changes is here: http://cr

Re: 8218559: Reimplement the Legacy Socket API

2019-05-09 Thread Michael McMahon
On 07/05/2019, 17:14, Alan Bateman wrote: On 07/05/2019 16:44, Michael McMahon wrote: Hi Alan, What's the purpose of the change to the UdpTest? The deprecated Socket constructors can be used to create a Socket that uses UDP. This means the new SocketImpl has to support the creation of

Re: 8218559: Reimplement the Legacy Socket API

2019-05-09 Thread Michael McMahon
Looks good to me. I don't have anything else to add at this point. I agree the new implementation is much clearer than the old and it will be great to see it working in the field. Thanks, Michael On 09/05/2019, 14:00, Alan Bateman wrote: Thanks Chris and Michael for the reviews/help so far. I'

RFR 8216417: cleanup of IPv6 scope-id handling

2019-06-10 Thread Michael McMahon
Hi, Could I get the following change to net/nio reviewed please? It is a general cleanup of IPv6 scope_id handling which removes a lot of native code trickery (mostly in Linux) and simplifies the handling of scope_ids such that: a) when binding/connecting/sending to a link-local address on the

Re: RFR 8216417: cleanup of IPv6 scope-id handling

2019-06-11 Thread Michael McMahon
Good point Daniel. Thanks for the review. - Michael. On 10/06/2019, 19:48, Daniel Fuchs wrote: Hi Michael, AbstractPlainDatagramSocketImpl.java: in `send` maybe the packet should only be recreated if the address is different. best regards, -- daniel On 10/06/2019 15:18, Michael McMahon

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

2019-06-11 Thread Michael McMahon
Looks like a useful improvement for test stabilization Daniel. Looks good to me. Michael. On 10/06/2019, 15:14, Daniel Fuchs wrote: Hi, Please find below a fix for: 8225512: Replace wildcard address with loopback or local host in tests - part 15 https://bugs.openjdk.java.net/browse/JD

Re: RFR 8216417: cleanup of IPv6 scope-id handling

2019-06-11 Thread Michael McMahon
15:18, Michael McMahon wrote: Hi, Could I get the following change to net/nio reviewed please? It is a general cleanup of IPv6 scope_id handling which removes a lot of native code trickery (mostly in Linux) and simplifies the handling of scope_ids such that: a) when binding/connecting/sending to a

Re: RFR 8216417: cleanup of IPv6 scope-id handling

2019-06-11 Thread Michael McMahon
, at 12:11, Michael McMahon wrote: Thanks for the review Alan. I've made the changes suggested by you and Daniel and added a test to Scoping.java for checking the connected IP address. http://cr.openjdk.java.net/~michaelm/8216417/webrev.3/index.html Looks like AbstractPlainDatagramSocke

Re: RFR 8216417: cleanup of IPv6 scope-id handling

2019-06-11 Thread Michael McMahon
http://cr.openjdk.java.net/~michaelm/8216417/webrev.4/ Thanks, Michael. On 11/06/2019, 13:19, Alan Bateman wrote: On 11/06/2019 12:11, Michael McMahon wrote: Thanks for the review Alan. I've made the changes suggested by you and Daniel and added a test to Scoping.java for checking the connected IP addr

RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

2019-06-21 Thread Michael McMahon
Small test case update. The test has failed a couple of times where it appears to be receiving input on a multicast socket which could not be generated by the test case itself. The test happens to use multicast groups that are assigned by IANA, and globally routable. So, it is conceivable that o

Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

2019-06-21 Thread Michael McMahon
Hi Chris, On 21/06/2019, 12:32, Chris Hegarty wrote: Michael, On 21/06/2019 11:53, Michael McMahon wrote: Small test case update. The test has failed a couple of times where it appears to be receiving input on a multicast socket which could not be generated by the test case itself. The test

Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

2019-06-24 Thread Michael McMahon
------ *From:* net-dev on behalf of Michael McMahon *Sent:* Friday 21 June 2019 16:27 *To:* Chris Hegarty *Cc:* OpenJDK Network Dev list *Subject:* Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException Hi Chris, On 21/06/2019, 12:32, Chri

Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

2019-06-24 Thread Michael McMahon
Hi Daniel, On 21/06/2019, 17:54, Daniel Fuchs wrote: Hi Michael, Thanks for doing that. IT should make this test much more stable. On 21/06/2019 17:27, Michael McMahon wrote: There is a nio test, java/nio/channels/DatagramChannel/Promiscuous.java that follows a similar pattern. Should it be

Re: RFR: 8219804 ava/net/MulticastSocket/Promiscuous.java fails intermittently due to NumberFormatException

2019-06-24 Thread Michael McMahon
On 24/06/2019, 11:31, Michael McMahon wrote: Hi Mark On 23/06/2019, 13:53, mark sheppard wrote: : ​ Since the socket is receiving something unexpected, I think I agree that binding to the multicast address could be worth doing as well. That would filter out packets that perhaps broadcast

Re: [BUG] Inet6Address.isIPv4CompatibleAddress uses wrong prefix

2019-06-25 Thread Michael McMahon
On 25/06/2019, 07:37, Alan Bateman wrote: On 24/06/2019 21:57, Rob Spoor wrote: I found a bug in Inet6Adress.isIPv4CompatibleAddress(). While parsing correctly uses the ::: format, isIPv4CompatibleAddress() checks for :: instead. The notion "IPv4-compatible IPv6 address" is different to "IP

RFR: 8222968 ByteArrayPublisher is not thread-safe resulting in broken re-use of HttpRequests

2019-06-27 Thread Michael McMahon
Could I get the following change reviewed please: The change is as suggested in the report and the test case is based on the one provided. http://cr.openjdk.java.net/~michaelm/8222968/webrev.1/index.html Thanks, Michael.

Re: RFR: 8222968 ByteArrayPublisher is not thread-safe resulting in broken re-use of HttpRequests

2019-06-27 Thread Michael McMahon
Hi Chris, On 27/06/2019, 11:50, Chris Hegarty wrote: Michael, On 27 Jun 2019, at 11:08, Michael McMahon wrote: Could I get the following change reviewed please: The change is as suggested in the report and the test case is based on the one provided. http://cr.openjdk.java.net/~michaelm

Re: RFR: 8225239: Refactor NetworkInterface lookups

2019-07-05 Thread Michael McMahon
Claes, This is great work and long overdue. Thanks for taking care of it. - Michael. On 04/07/2019, 20:20, Claes Redestad wrote: Hi, please review this patch that refactors native java.net.NetworkInterface lookup logic in a few ways to address both pre-existing and recent regressions: Bug:

RFR: 8225479 com.sun.net.httpserver.HttpContext that does not end with '/' has surprising matches

2019-07-09 Thread Michael McMahon
Could I get the following small doc clarification reviewed please? http://cr.openjdk.java.net/~michaelm/8225479/webrev.1/ Thanks Michael

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

2019-07-10 Thread Michael McMahon
Hi Daniel, I think I prefer the pattern of URL creation where you use URIBuilder and then toURL() as the final step, rather than toString() and then new URL(string). But, it's not a big deal. NoLoopbackPackets looks like it could be simplified with try with resources (line 166) unless you exp

Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

2019-07-24 Thread Michael McMahon
Hi Jaikiran, Thanks for looking at this issue. ProxySelector::select is not a particularly well specified method. The spec is a little vague on what is expected of the supplied URI parameter. It seems to me however, that the intent is that the supplied URI should contain a valid protocol (scheme

Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

2019-07-30 Thread Michael McMahon
Hi Jaikiran, On 29/07/2019, 13:12, Jaikiran Pai wrote: Hello Michael, Thank you for reviewing this. Comments inline. On 24/07/19 10:37 PM, Michael McMahon wrote: Hi Jaikiran, Thanks for looking at this issue. ProxySelector::select is not a particularly well specified method. The spec is a

Re: RFR: 8185898: setRequestProperty(key, null) results in HTTP header without colon in request

2019-07-31 Thread Michael McMahon
Hi Julia, I will sponsor this for you. Trivially, there are a couple of unnecessary blank lines inserted at the top of MessageHeader.java. I think the @summary in the new test might be better just having the original bug synopsis in this case. Otherwise, looks fine. Thanks, Michael. On 31/0

Re: RFR: 8185898: setRequestProperty(key, null) results in HTTP header without colon in request

2019-07-31 Thread Michael McMahon
Daniel I don't think the change affects the usage of response headers, but it might be useful to include in the test a call to getHeaderField(0) to verify that. Michael. On 31/07/2019, 12:30, Daniel Fuchs wrote: Hi Julia, Could you verify that `HttpURLConnection::getHeaderField(0)` and `HttpU

Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

2019-08-01 Thread Michael McMahon
CSR for this, but it could be next week before I get around to that. Thanks Michael. On 01/08/2019, 08:03, Jaikiran Pai wrote: Hello Michael, On 31/07/19 11:43 AM, Michael McMahon wrote: ... I wonder if another solution is possible where the IAE is caught at the appropriate place(s) and

RFR 8199849: HttpServer/BasicAuthenticator: unicode bytes are not correctly handled

2019-08-06 Thread Michael McMahon
This change went through a couple of rounds of review before, and I got diverted to something else at the time. So, just picking it up now. Bug: https://bugs.openjdk.java.net/browse/JDK-8199849 Webrev: http://cr.openjdk.java.net/~michaelm/8199849/webrev.3/index.html CSR: https://bugs.openjdk.j

Re: RFR 8199849: HttpServer/BasicAuthenticator: unicode bytes are not correctly handled

2019-08-06 Thread Michael McMahon
please? it's a bit confusing given that the server authenticator is a stored in a variable named `testAuthenticator` as well. Otherwise, looks good to me! best regards, -- daniel On 06/08/2019 12:44, Michael McMahon wrote: This change went through a couple of rounds of review before, an

Re: RFR 8199849: HttpServer/BasicAuthenticator: unicode bytes are not correctly handled

2019-08-12 Thread Michael McMahon
I have made some more changes and there is a new webrev at: http://cr.openjdk.java.net/~michaelm/8199849/webrev.5/ I would like to finalise the CSR soon also. So, any further comments appreciated. Thanks, Michael On 06/08/2019, 17:20, Michael McMahon wrote: Thanks Daniel. Suggested changes

Re: [teststabilization] RFR: 8229481: sun/net/www/protocol/https/ChunkedOutputStream.java failed with a SSLException

2019-08-15 Thread Michael McMahon
Would it be clearer if test0() of chunkedoutputstream was renamed to testPlainText() and it is given the authority component only. So, it would create the URL itself and it would be clearer that it is creating a http url? Minor comment typo in "veryfy" Otherwise, looks good. - Michael. On

RFR: 8222363 Update ServerSocket.isBound spec to reflect implementation after close

2019-08-21 Thread Michael McMahon
Could I get the following trivial doc change reviewed please? Bug report: https://bugs.openjdk.java.net/browse/JDK-8222363 Webrev: http://cr.openjdk.java.net/~michaelm/8222363/webrev.1/index.html CSR: https://bugs.openjdk.java.net/browse/JDK-8229978 Thanks, Michael.

Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

2019-08-22 Thread Michael McMahon
or host cannot be determined from the provided + * {@code uri} ||| I changed the suggested wording slightly as follows: 'from the provided {@code uri}' Thanks, Michael. || On 01/08/2019, 11:52, Michael McMahon wrote: Hi Jaikiran, This looks good to me. There are a

Re: RFR 6563286 6797318 8177648 - Undeclared IAE thrown from HttpURLConnection.connect for some URLs

2019-08-22 Thread Michael McMahon
Jaikiran It's not necessary to update the webrev with the change I made. I've put it into my copy of the patch already. Thanks Michael On 22/08/2019, 17:19, Jaikiran Pai wrote: On 22/08/19 8:17 PM, Michael McMahon wrote: Getting back to this issue. I have filed a CS

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