Re: [jdk17] RFR: 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1
On Thu, 10 Jun 2021 17:39:58 GMT, Daniel Fuchs wrote: > Another change brought by jtreg 6+1 in the behavior of @BeforeMethod. > Now throwing an exception in @BeforeMethod causes all annotated methods to be > skipped, not just those annotated with @Test. > > HttpClient tests that use @BeforeMethod and ITestContext need to be updated > to work around the new behavior. > > Rebased to jdk17 repo. Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk17/pull/5
[jdk17] Integrated: 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1
On Thu, 10 Jun 2021 17:39:58 GMT, Daniel Fuchs wrote: > Another change brought by jtreg 6+1 in the behavior of @BeforeMethod. > Now throwing an exception in @BeforeMethod causes all annotated methods to be > skipped, not just those annotated with @Test. > > HttpClient tests that use @BeforeMethod and ITestContext need to be updated > to work around the new behavior. > > Rebased to jdk17 repo. This pull request has now been integrated. Changeset: da043e99 Author:Daniel Fuchs URL: https://git.openjdk.java.net/jdk17/commit/da043e99b830fa4fcbfdbdbed182abc394ba6fb1 Stats: 304 lines in 10 files changed: 276 ins; 0 del; 28 mod 8268555: Update HttpClient tests that use ITestContext to jtreg 6+1 Reviewed-by: chegar - PR: https://git.openjdk.java.net/jdk17/pull/5
Unix Domain Socket with Websocket
Hello all, we've a native application (in C) that exposes a json-rpc interface over a Unix Domain Socket using websocket. I know that the upcoming Java 17 release will include support for unix domain sockets and I'm wondering if the included websocket client (since Java 11) will also support this mechanism ? If not planned, any advice on how you can achieve this today ? * me hopes that I don't need to touch C code and continue with Java :-) Kind regards, -Christos
Re: Unix Domain Socket with Websocket
Hi Christos, The HttpClient doesn't support Unix Domain Socket - only regular TCP / TLS. You could of course open a connection with your client using a plain Unix Domain SocketChannel [1] using the UNIX ProtocolFamilly [2]. best regard, -- daniel [1] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/nio/channels/SocketChannel.html#open(java.net.ProtocolFamily) [2] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/net/StandardProtocolFamily.html On 11/06/2021 09:57, Christos Vasilakis wrote: Hello all, we've a native application (in C) that exposes a json-rpc interface over a Unix Domain Socket using websocket. I know that the upcoming Java 17 release will include support for unix domain sockets and I'm wondering if the included websocket client (since Java 11) will also support this mechanism ? If not planned, any advice on how you can achieve this today ? * me hopes that I don't need to touch C code and continue with Java :-) Kind regards, -Christos
Re: Unix Domain Socket with Websocket
Yes, it does seem that for local RPC a regular (unix domain) socket should suffice rather than a websocket. Also, just to point out that Unix domain socket (channels) have been in the JDK since Java 16. - Michael. On 11/06/2021 10:22, Daniel Fuchs wrote: Hi Christos, The HttpClient doesn't support Unix Domain Socket - only regular TCP / TLS. You could of course open a connection with your client using a plain Unix Domain SocketChannel [1] using the UNIX ProtocolFamilly [2]. best regard, -- daniel [1] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/nio/channels/SocketChannel.html#open(java.net.ProtocolFamily) [2] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/net/StandardProtocolFamily.html On 11/06/2021 09:57, Christos Vasilakis wrote: Hello all, we've a native application (in C) that exposes a json-rpc interface over a Unix Domain Socket using websocket. I know that the upcoming Java 17 release will include support for unix domain sockets and I'm wondering if the included websocket client (since Java 11) will also support this mechanism ? If not planned, any advice on how you can achieve this today ? * me hopes that I don't need to touch C code and continue with Java :-) Kind regards, -Christos
RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream
@dfuch could you please review, thank you. - Commit messages: - 8263364: fixed headers - JDK-8263364: initial commit Changes: https://git.openjdk.java.net/jdk/pull/4472/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4472&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263364 Stats: 67 lines in 1 file changed: 38 ins; 5 del; 24 mod Patch: https://git.openjdk.java.net/jdk/pull/4472.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4472/head:pull/4472 PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream
On Fri, 11 Jun 2021 10:46:32 GMT, Ivan Šipka wrote:
> @dfuch could you please review, thank you.
Globally looks good - still need some cleanup to make sure everything gets
closed in the end.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 56:
> 54:
> 55: ByteArrayOutputStream clientBytes;
> 56: Socket socket = null;
socket should probably be a volatile field in XServer si that it can be closed
at the end of main()
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 118:
> 116:
> 117: try {
> 118: XServer server = new XServer(serversocket);
You should take that statement outside of the try block
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 142:
> 140: } catch (NullPointerException e) {
> 141: throw new RuntimeException (e);
> 142: } finally {
The finally block should also do:
var socket = server.socket;
if (socket != null) socket.close();
Or better add a close() method on XServer that would both close socket and
serverSocket - and make XServer AutoCloseable so that you can transform your
try { } finally {} into a try-with-resource.
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream
On Fri, 11 Jun 2021 10:46:32 GMT, Ivan Šipka wrote:
> @dfuch could you please review, thank you.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 139:
> 137: is.close();
> 138: } catch (IOException e) {
> 139: throw new RuntimeException (e);
No need to catch and wrap since main is declared to throw Exception. Just let
IOException and NullPointerException percolate out of main...
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests [v2]
> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests Mahendra Chhipa has updated the pull request incrementally with one additional commit since the last revision: Implemented review comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4432/files - new: https://git.openjdk.java.net/jdk/pull/4432/files/b490e1ff..49965732 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4432&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4432&range=00-01 Stats: 36 lines in 2 files changed: 5 ins; 17 del; 14 mod Patch: https://git.openjdk.java.net/jdk/pull/4432.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4432/head:pull/4432 PR: https://git.openjdk.java.net/jdk/pull/4432
Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests [v3]
> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests Mahendra Chhipa has updated the pull request incrementally with one additional commit since the last revision: Remove extra space. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4432/files - new: https://git.openjdk.java.net/jdk/pull/4432/files/49965732..db615030 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4432&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4432&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4432.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4432/head:pull/4432 PR: https://git.openjdk.java.net/jdk/pull/4432
Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests [v3]
On Fri, 11 Jun 2021 13:38:14 GMT, Mahendra Chhipa
wrote:
>> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests
>
> Mahendra Chhipa has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Remove extra space.
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/B6216082.java line 89:
> 87: // created as it will use an ephemeral port.
> 88: System.setProperty("https.proxyPort",
> 89: Integer.toString(proxy.getLocalPort()));
A potentially better alternative to setting system properties in main could be
to set a ProxySelector (using ProxySelector.setDefault());
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/B6216082.java line 184:
> 182: HttpURLConnection uc = (HttpURLConnection)url.openConnection();
> 183: System.out.println(uc.getResponseCode());
> 184: if(uc.getResponseCode() == 400) {
It could be better to use: `uc.getResponseCode() != 200` here - since anything
but 200 should be an error?
test/jdk/sun/net/www/protocol/https/HttpsURLConnection/B6216082.java line 186:
> 184: if(uc.getResponseCode() == 400) {
> 185: uc.disconnect();
> 186: throw new RuntimeException("Test failed : bad http request");
Maybe add the response code to the exception message.
-
PR: https://git.openjdk.java.net/jdk/pull/4432
Re: Unix Domain Socket with Websocket
Thank you all for your replies. Kind regards -Christos On Fri, Jun 11, 2021 at 12:50 PM Michael McMahon < michael.x.mcma...@oracle.com> wrote: > Yes, it does seem that for local RPC a regular (unix domain) socket should > suffice > rather than a websocket. > > Also, just to point out that Unix domain socket (channels) have been in > the JDK > since Java 16. > > - Michael. > On 11/06/2021 10:22, Daniel Fuchs wrote: > > Hi Christos, > > The HttpClient doesn't support Unix Domain Socket - only regular > TCP / TLS. > > > You could of course open a connection with your client using > a plain Unix Domain SocketChannel [1] using the UNIX > ProtocolFamilly [2]. > > best regard, > > -- daniel > [1] > https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/nio/channels/SocketChannel.html#open(java.net.ProtocolFamily) > [2] > https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/net/StandardProtocolFamily.html > > On 11/06/2021 09:57, Christos Vasilakis wrote: > > Hello all, > > we've a native application (in C) that exposes a json-rpc interface over a > Unix Domain Socket using websocket. I know that the upcoming Java 17 > release will include support for unix domain sockets and I'm wondering if > the included websocket client (since Java 11) will also support this > mechanism ? > > If not planned, any advice on how you can achieve this today ? > > * me hopes that I don't need to touch C code and continue with Java :-) > > Kind regards, > -Christos > > > > >
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]
> @dfuch could you please review, thank you. Ivan Šipka has updated the pull request incrementally with one additional commit since the last revision: 8263364: refactor - Changes: - all: https://git.openjdk.java.net/jdk/pull/4472/files - new: https://git.openjdk.java.net/jdk/pull/4472/files/84653b50..f4ea6c84 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4472&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4472&range=00-01 Stats: 49 lines in 1 file changed: 18 ins; 15 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/4472.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4472/head:pull/4472 PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]
On Fri, 11 Jun 2021 10:58:42 GMT, Daniel Fuchs wrote:
>> Ivan Šipka has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8263364: refactor
>
> test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
> line 56:
>
>> 54:
>> 55: ByteArrayOutputStream clientBytes;
>> 56: Socket socket = null;
>
> socket should probably be a volatile field in XServer si that it can be
> closed at the end of main()
it closes it any case but since the RW operations and close operations are in
different threads, I have added the volatile keywords.
> test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
> line 139:
>
>> 137: is.close();
>> 138: } catch (IOException e) {
>> 139: throw new RuntimeException (e);
>
> No need to catch and wrap since main is declared to throw Exception. Just let
> IOException and NullPointerException percolate out of main...
percolate <3
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]
On Fri, 11 Jun 2021 14:42:14 GMT, Ivan Šipka wrote:
>> @dfuch could you please review, thank you.
>
> Ivan Šipka has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8263364: refactor
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 45:
> 43: static class XServer extends Thread implements AutoCloseable {
> 44:
> 45: volatile ServerSocket serverSocket;
could be final instead of volatile
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 112:
> 110: @Override
> 111: public void close() throws Exception {
> 112: clientSocket.close();
you should really guard against null here (using a local variable to capture
clientSocket) since clientSocket could be transiently null until the run()
method is called.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 120:
> 118: public static void main (String[] args) throws Exception {
> 119:
> 120: final Integer port = 61234;
Why suddenly use a known port? That's a recipe for intermittent test failures.
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 147:
> 145: e.printStackTrace();
> 146: }
> 147:
Why catch exception? That will hide test failures. Let exceptions percolate out
of main instead.
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]
On Fri, 11 Jun 2021 14:48:56 GMT, Daniel Fuchs wrote:
>> Ivan Šipka has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8263364: refactor
>
> test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
> line 120:
>
>> 118: public static void main (String[] args) throws Exception {
>> 119:
>> 120: final Integer port = 61234;
>
> Why suddenly use a known port? That's a recipe for intermittent test failures.
it was mentioned in the last comment to use the combination of the specific
port and URL, but that is likely a lapsus. will change now
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v3]
> @dfuch could you please review, thank you. Ivan Šipka has updated the pull request incrementally with one additional commit since the last revision: 8263364: refactor - Changes: - all: https://git.openjdk.java.net/jdk/pull/4472/files - new: https://git.openjdk.java.net/jdk/pull/4472/files/f4ea6c84..4f530cea Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4472&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4472&range=01-02 Stats: 17 lines in 1 file changed: 7 ins; 4 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/4472.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4472/head:pull/4472 PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v2]
On Fri, 11 Jun 2021 15:19:49 GMT, Ivan Šipka wrote:
>> test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
>> line 120:
>>
>>> 118: public static void main (String[] args) throws Exception {
>>> 119:
>>> 120: final Integer port = 61234;
>>
>> Why suddenly use a known port? That's a recipe for intermittent test
>> failures.
>
> it was mentioned in the last comment to use the combination of the specific
> port and URL, but that is likely a lapsus. will change now
come to think of it, I completely misunderstood it.
-
PR: https://git.openjdk.java.net/jdk/pull/4472
Re: RFR: 8263364: sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java wedged in getInputStream [v3]
On Fri, 11 Jun 2021 15:41:18 GMT, Ivan Šipka wrote:
>> @dfuch could you please review, thank you.
>
> Ivan Šipka has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8263364: refactor
Looks good - small change still needed in `XServer::close`
test/jdk/sun/net/www/http/KeepAliveStream/KeepAliveStreamCloseWithWrongContentLength.java
line 118:
> 116: public void close() throws Exception {
> 117: if (clientSocket != null) {
> 118: clientSocket.close();
Given that clientSocket is volatile you should really capture its value in a
local variable:
var clientSocket = this.clientSocket;
if (clientSocket != null) {
clientSocket.close();
-
PR: https://git.openjdk.java.net/jdk/pull/4472
