fix fabric8io#4569: accounting for 0 timeouts that are not supported …

…by jdk

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

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) {

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<WebSocketResponse> 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();

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;

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());
+    }
+  }
+
+}

kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/http/AbstractWebSocketSendTest.java

@@ -62,14 +62,18 @@ void sendEmitsMessageToWebSocketServer() throws Exception {
           .done()
           .always();
       final BlockingQueue<String> 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
           //   directly
           //.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));
             }