Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
On Tue, 1 Mar 2022 17:09:57 GMT, zzambers wrote: > Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 Please also update JDK-8282529 to include the description (i.e. above). IMHO, the bug should contain the information for the issue, rather than need to navigate 2 steps to the pull request. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
On Tue, 1 Mar 2022 17:09:57 GMT, zzambers wrote: > Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 Please update the copyright to include 2022. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
On Tue, 1 Mar 2022 17:09:57 GMT, zzambers wrote: > Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 Thank you for the nice catch. The update looks good to me. A CSR may be required for spec change. - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: JDK-8282354 : Remove dependancy of TestHttpServer, HttpTransaction, HttpCallback from open/test/jdk/ tests [v3]
On Fri, 25 Feb 2022 15:06:29 GMT, Mahendra Chhipa wrote:
>> Updated following remaining tests to remove depenedies of TestHttpServer,
>> HttpTransaction, HttpCallback
>> open/test/jdk/java/net/ProxySelector/LoopbackAddresses.java
>> open/test/jdk/java/net/ProxySelector/ProxyTest.java
>> open/test/jdk/java/net/URL/PerConnectionProxy.java
>> open/test/jdk/java/net/URLConnection/B5052093.java
>> open/test/jdk/sun/net/www/AuthHeaderTest.java
>> open/test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java
>
> Mahendra Chhipa has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed extra whitespace
test/jdk/java/net/ProxySelector/LoopbackAddresses.java line 168:
> 166: try(PrintWriter pw = new
> PrintWriter(exchange.getResponseBody())) {
> 167: pw.print("Hello .");
> 168: }
I know that now UTF-8 is supposed to be the default - but I'd prefer to either
make it explicit, or add a comment stating that since Java 18 PrintWriter will
use UTF-8 encoding by default.
test/jdk/java/net/ProxySelector/ProxyTest.java line 116:
> 114: e.printStackTrace();
> 115: }
> 116: try(PrintWriter pw = new
> PrintWriter(exchange.getResponseBody())) {
Same remark here
test/jdk/java/net/URL/PerConnectionProxy.java line 234:
> 232: } catch (IOException e) {
> 233: }
> 234: try(PrintWriter pw = new
> PrintWriter(exchange.getResponseBody())) {
And here too
test/jdk/java/net/URLConnection/B5052093.java line 113:
> 111: exchange.close();
> 112: } catch (IOException e) {
> 113: e.printStackTrace();
Are you sure that this results in the same response headers than before?
If I'm not mistaken here we will send both Content-Length and
Transfer-Encoding: chunked. Was that what the previous server did, and what the
test wants to test?
test/jdk/sun/net/www/AuthHeaderTest.java line 139:
> 137: void okReply (HttpExchange req) throws IOException {
> 138: req.sendResponseHeaders (200, 0);
> 139: try(PrintWriter pw = new PrintWriter(req.getResponseBody())) {
Same remark about UTF-8
test/jdk/sun/net/www/AuthHeaderTest.java line 167:
> 165: }
> 166: }
> 167: }
missing newline at end of file?
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 157:
> 155: }
> 156:
> 157: class SimpleHttpTransactionHandler implements HttpHandler
the boolean `failed` should at least be volatile
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 178:
> 176: responseBody[i] = 0x41;
> 177: trans.sendResponseHeaders(200, 0);
> 178: try(PrintWriter pw = new
> PrintWriter(trans.getResponseBody())) {
Same remark about UTF-8 here again
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 206:
> 204: // override the Content-length header to be greater than
> the actual response body
> 205: trans.getResponseHeaders().set("Content-length",
> Integer.toString(responseBody.length+1));
> 206: trans.sendResponseHeaders(200, 0);
Here again we will be mixing Content-Length and chunked
test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 207:
> 205: trans.getResponseHeaders().set("Content-length",
> Integer.toString(responseBody.length+1));
> 206: trans.sendResponseHeaders(200, 0);
> 207: try(PrintWriter pw = new
> PrintWriter(trans.getResponseBody())) {
Same remark for UTF-8
-
PR: https://git.openjdk.java.net/jdk/pull/7616
RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / Socket.shutdownOutput() and InputStream.close() / OutputStream.close() performed half-close of TLS-1.3 connection. However this behaviour has changed as result of JDK-8216326 [2]. InputStream.close() / OutputStream.close() no longer perform half-close but full socket close, but API Note was never updated. [1] https://bugs.openjdk.java.net/browse/JDK-8208526 [2] https://bugs.openjdk.java.net/browse/JDK-8216326 - Commit messages: - javax/net/ssl/SSLSocket: Fixed API Note in javadoc Changes: https://git.openjdk.java.net/jdk/pull/7648/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7648=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282529 Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7648.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7648/head:pull/7648 PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
On Tue, 1 Mar 2022 17:09:57 GMT, zzambers wrote: > Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was > introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / > Socket.shutdownOutput() and InputStream.close() / OutputStream.close() > performed half-close of TLS-1.3 connection. However this behaviour has > changed as result of JDK-8216326 [2]. InputStream.close() / > OutputStream.close() no longer perform half-close but full socket close, but > API Note was never updated. > > [1] https://bugs.openjdk.java.net/browse/JDK-8208526 > [2] https://bugs.openjdk.java.net/browse/JDK-8216326 @zzambers I've created a corresponding issue for you in the tracker, please rename your PR title to 8282529 and the system should automatically mark your PR as ready for review - PR: https://git.openjdk.java.net/jdk/pull/7648
Re: RFR: 8282529: Fix API Note in javadoc for javax.net.ssl.SSLSocket
On Wed, 2 Mar 2022 07:13:36 GMT, TheShermanTanker wrote: >> Fixed API Note in javadoc for javax.net.ssl.SSLSocket class. API Note was >> introduced by JDK-8208526 [1]. At that point both Socket.shutdownInput() / >> Socket.shutdownOutput() and InputStream.close() / OutputStream.close() >> performed half-close of TLS-1.3 connection. However this behaviour has >> changed as result of JDK-8216326 [2]. InputStream.close() / >> OutputStream.close() no longer perform half-close but full socket close, but >> API Note was never updated. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8208526 >> [2] https://bugs.openjdk.java.net/browse/JDK-8216326 > > @zzambers I've created a corresponding issue for you in the tracker, please > rename your PR title to 8282529 and the system should automatically mark your > PR as ready for review @TheShermanTanker thank you - PR: https://git.openjdk.java.net/jdk/pull/7648
