diff --git a/CHANGELOG.md b/CHANGELOG.md index 4918b93b3bb..af4e49e7d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Fix #4491: added a more explicit shutdown exception for okhttp * Fix #4534: Java Generator CLI default handling of skipGeneratedAnnotations * Fix #4535: The shell command string will now have single quotes sanitized +* Fix #4569: fixing jdk httpclient regression with 0 timeouts * Fix #4547: preventing timing issues with leader election cancel #### Improvements diff --git a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilderImpl.java b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilderImpl.java index a89fc687ed9..38513505897 100644 --- a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilderImpl.java +++ b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilderImpl.java @@ -53,7 +53,7 @@ public HttpClient build() { return new JdkHttpClientImpl(this, client.getHttpClient(), this.requestConfig); } java.net.http.HttpClient.Builder builder = clientFactory.createNewHttpClientBuilder(); - if (connectTimeout != null) { + if (connectTimeout != null && !java.time.Duration.ZERO.equals(connectTimeout)) { builder.connectTimeout(connectTimeout); } if (sslContext != null) { diff --git a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java index a6c186293bf..db0ad225c7f 100644 --- a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java +++ b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientImpl.java @@ -35,6 +35,7 @@ import java.net.http.WebSocketHandshakeException; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.List; import java.util.Map; import java.util.Optional; @@ -404,8 +405,9 @@ public CompletableFuture internalBuildAsync(JdkWebSocketImpl. } // the Watch logic sets a websocketTimeout as the readTimeout // TODO: this should probably be made clearer in the docs - if (this.builder.getReadTimeout() != null) { - newBuilder.connectTimeout(this.builder.getReadTimeout()); + Duration readTimeout = this.builder.getReadTimeout(); + if (readTimeout != null && !java.time.Duration.ZERO.equals(readTimeout)) { + newBuilder.connectTimeout(readTimeout); } AtomicLong queueSize = new AtomicLong(); diff --git a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpRequestImpl.java b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpRequestImpl.java index fc130c074a2..c0e5b46f846 100644 --- a/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpRequestImpl.java +++ b/httpclient-jdk/src/main/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpRequestImpl.java @@ -115,7 +115,7 @@ public Builder setHeader(String k, String v) { } public Builder timeout(Duration duration) { - if (duration != null) { + if (duration != null && !Duration.ZERO.equals(duration)) { builder.timeout(duration); } return this; diff --git a/httpclient-jdk/src/test/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilder.java b/httpclient-jdk/src/test/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilder.java new file mode 100644 index 00000000000..e311de8c29c --- /dev/null +++ b/httpclient-jdk/src/test/java/io/fabric8/kubernetes/client/jdkhttp/JdkHttpClientBuilder.java @@ -0,0 +1,41 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.jdkhttp; + +import io.fabric8.kubernetes.client.http.HttpClient; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertNotNull; + +class JdkHttpClientBuilder { + + @Test + void testZeroTimeouts() { + JdkHttpClientFactory factory = new JdkHttpClientFactory(); + JdkHttpClientBuilderImpl builder = factory.newBuilder(); + + // should build and be usable without an issue + try (HttpClient client = builder.readTimeout(0, TimeUnit.MILLISECONDS).connectTimeout(0, TimeUnit.MILLISECONDS) + .writeTimeout(0, + TimeUnit.MILLISECONDS) + .build();) { + assertNotNull(client.newHttpRequestBuilder().uri("http://localhost").build()); + } + } + +} diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractWebSocketSendTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractWebSocketSendTest.java index 69a35845d0c..2f5eaa453ed 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractWebSocketSendTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractWebSocketSendTest.java @@ -62,7 +62,10 @@ void sendEmitsMessageToWebSocketServer() throws Exception { .done() .always(); final BlockingQueue receivedText = new ArrayBlockingQueue<>(1); - final WebSocket ws = client.newWebSocketBuilder() + final WebSocket ws = client + // ensure that both a derived builder and a 0, or no, timeout works + // as that is a common logic path in the client + .newBuilder().readTimeout(0, TimeUnit.SECONDS).build().newWebSocketBuilder() // TODO: JDK HttpClient implementation doesn't work with ws URIs // - Currently we are using an HttpRequest.Builder which is then // mapped to a WebSocket.Builder. We should probably user the WebSocket.Builder @@ -70,6 +73,7 @@ void sendEmitsMessageToWebSocketServer() throws Exception { //.uri(URI.create(String.format("ws://%s:%s/send-text", server.getHostName(), server.getPort()))) .uri(URI.create(server.url("send-text"))) .buildAsync(new WebSocket.Listener() { + @Override public void onMessage(WebSocket webSocket, String text) { assertTrue(receivedText.offer(text)); }