RFR JDK-8240921: Minor correction to HttpResponse.BodySubscribers example

[rahul . r . yadav]

Hello,

 Can I please have my fix reviewed for issue JDK-8240921 - Minor 
correction to HttpResponse.BodySubscribers example?


 The fix updates the incorrect example of the usage of the class 
java.net.http.HttpResponse.BodySubscribers.The patch also updates the 
test case for this scenario.


 Issue: https://bugs.openjdk.java.net/browse/JDK-8240921

 Webrev: 
http://cr.openjdk.java.net/~pconcannon/rayayada/8240921/webrev/webrev.00,


- Rahul

RFR JDK-8240921: Minor correction to HttpResponse.BodySubscribers example

[rahul . r . yadav]

Hello,

 Can I please have my fix reviewed for issue JDK-8240921 - Minor 
correction to HttpResponse.BodySubscribers example?


 The fix updates the incorrect example of the usage of the class 
java.net.http.HttpResponse.BodySubscribers.The patch also updates the 
test case for this scenario.


 Issue: https://bugs.openjdk.java.net/browse/JDK-8240921

 Webrev: 
http://cr.openjdk.java.net/~pconcannon/rayayada/8240921/webrev/webrev.00 



- Rahul

RFR JDK-8240921: Minor correction to HttpResponse.BodySubscribers example

[rahul . r . yadav]

Hello,

 Can I please have my fix reviewed for issue JDK-8240921 - Minor 
correction to HttpResponse.BodySubscribers example?


 The fix updates the incorrect example of the usage of the class 
java.net.http.HttpResponse.BodySubscribers.The patch also updates the 
test case for this scenario.


 Issue: https://bugs.openjdk.java.net/browse/JDK-8240921

 Webrev: 
http://cr.openjdk.java.net/~pconcannon/rayayada/8240921/webrevs/webrev.00/


- Rahul

RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

[rahul . r . yadav]

Hello,

Request to have my fix reviewed for issues:

    JDK-8239595 : ssl context version is not respected
    JDK-8239594 : jdk.tls.client.protocols is not respected

The fix updates 
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of 
ctx.getSupportedSSLParameters(),

as the latter does not respect the context parameters set by the user.

Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594

Webrev: 
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/


-- Rahul

Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected

[rahul . r . yadav]

The current fix does not affect the scenarios discussed earlier(that is 
a broader discussion,may be a different bug/enhancement).

The scenarios would be vaild even if the fix would not have been in place.

-Rahul

On 27/03/2020 17:50, Chris Hegarty wrote:
Thank you for these clarifications. We will now consider how these 
affect, if at all, the HTTP Client.


-Chris.

On 27 Mar 2020, at 17:47, Xuelei Fan > wrote:


On 3/27/2020 10:36 AM, Chris Hegarty wrote:

Thank you Xuelei, this very helpful.
Sorry, but I am going to ask just a few more clarifying questions to 
make sure that we’re on the same page.
On 27 Mar 2020, at 16:23, Xuelei Fan > wrote:


On 3/27/2020 5:52 AM, Chris Hegarty wrote:

Xuelei,
Before commenting further on the interaction of the HTTP Client 
with various contorted configurations, I would like to get a 
better understanding of the `jdk.tls.client.protocols` property.
Is there a specification or other documentation describing 
`jdk.tls.client.protocols` ?
See the jdk.tls.client.protocols line in table 'Table 8-3 System 
Properties and Customized Items" in JSSE Reference Guides:


"https://docs.oracle.com/en/java/javase/14/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-A41282C3-19A3-400A-A40F-86F4DA22ABA9

For your quick reference, I copied the note here:

---
Customized Item:
Default handshaking protocols for TLS/DTLS clients.

Notes:
To enable specific SunJSSE protocols on the client, specify them in 
a comma-separated list within quotation marks; all other supported 
protocols are not enabled on the client
“supported” here means protocols that are supported by the provider, 
and may be used within a specific context. This translates, for the 
default SSLContext, to the API call 
getSupportedSSLParameters().getProtocols(), right?

Yes.

getSupportedSSLParameters().getProtocols() returns a superset of 
getDefaultSSLParameters().getProtocols(). Conversely, 
getDefaultSSLParameters().getProtocols() is a strict subset of 
getSupportedSSLParameters().getProtocols(), right?

Yes.

The `jdk.tls.client.protocols` property has no affect on 
getSupportedSSLParameters().getProtocols()  only 
getDefaultSSLParameters().getProtocols(), right?

Yes.

In which case, getDefaultSSLParameters().getProtocols() returns the 
value of  `jdk.tls.client.protocols`.

For example,

   If jdk.tls.client.protocols="TLSv1,TLSv1.1", then the default 
protocol settings on the client for TLSv1 and TLSv1.1 are enabled, 
while SSLv3, TLSv1.2, TLSv1.3, and SSLv2Hello are not enabled


   If jdk.tls.client.protocols="DTLSv1.2" , then the protocol 
setting on the client for DTLS1.2 is enabled, while DTLS1.0 is not 
enabled

---
Seems that the term “client” here is referring to client-initiated 
exchanges, rather than any specific technology.
The assumption, which is reasonable, is that “clients” will use the 
default context. Again, this is reasonable default out-of-the-box 
behavior.
The client refer to the client side SSLSocket or SSLEngine created 
with the default SSLContext.  or example:

   SSLContext sslContext = SSLContext.getInstance("TLS");
   SSLEngine sslEngine = sslContext.createSSLEngine();
   sslEngine.setUseClientMode(true);

The sslEngine object is a client that impacted by the property.

While if
   sslEngine.setUseClientMode(false);

then the object should not be impacted by the property.

Xuelei

It is my understanding that the property only affects the 
*default* protocol’s ( not the supported protocols ) of the 
*default* context. That is, the context returned by 
`SSLContext.getInstance("Default”)`,
It is correct that the property impact the default SSLContext only. 
 The default SSLContext instance could get from:

   SSLContext.getInstance("Default");
   SSLContext.getInstance("TLS");
   SSLContext.getInstance("DTLS”);

Thanks for this clarification.


and the protocol values returned by the following invocation on 
that context `getDefaultSSLParameters().getProtocols()`. Is this 
correct? If not, what does it do?

Yes.

Thanks,
-Chris.

Re: RFR 8240666: Websocket client’s OpeningHandshake discards the HTTP response body

[rahul . r . yadav]

Thanks Daniel , webrev updated.

- rahul

On 06/05/2020 16:16, Daniel Fuchs wrote:

Hi Rahul,

LGTM.

 111 WebSocketHandshakeException wse = 
(WebSocketHandshakeException) t;
 112 out.println("Status code is " + 
wse.getResponse().statusCode());
 113 out.println("Response is " + 
wse.getResponse().body());

 114 assertNotNull(wse.getResponse());


I'd suggest moving line 114 two lines up (just after line 111)
to avoid triggering a NPE if wse.getResponse() returns null.
No need for a new webrev if that's the only change.

best regards,

-- daniel

On 06/05/2020 16:04, Rahul wrote:

Hi Pavel,

Thank you for the comment, the webrev has been updated.

webrev : 
http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html


- rahul

On 06/05/2020, 14:26, "Pavel Rappo"  wrote:

 An assertion of the form
  assertEquals(true, 
((String)wse.getResponse().body()).contains("404"));

  looks odd. I'd suggest using any of
  assertTrue(boolean condition)
 assertTrue(boolean condition, String message)
  -Pavel
  > On 6 May 2020, at 14:12, Rahul  
wrote:

 >
 > 
http://cr.openjdk.java.net/~ryadav/webrev_8240666/webrev.00/index.html