From 3c87d622d95c5b11fc3c97e19826256adf05280b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 15:21:11 +0000 Subject: [PATCH 1/4] Implement HTTP retry logic and set connection timeout in RESTClient - Implemented retry logic in `RESTClient` for `SocketTimeoutException`: up to 3 attempts with a 5-second pause between attempts. - Set the default connection timeout to 30 seconds. - Refactored `RESTClient.query` into `query` (retry loop) and `queryOnce` (single request logic). - Added `openConnection(URL)` and `sleep(long)` methods to `RESTClient` to facilitate testing. - Added `net.jsign.jca.RESTClientTest` to verify the retry logic and timeout. Co-authored-by: ebourg <54304+ebourg@users.noreply.github.com> --- .../main/java/net/jsign/jca/RESTClient.java | 32 ++++- .../java/net/jsign/jca/RESTClientTest.java | 115 ++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java diff --git a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java index a3d5cb8f..5af95b83 100644 --- a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java +++ b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.net.HttpURLConnection; +import java.net.SocketTimeoutException; import java.net.URL; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -124,9 +125,38 @@ public RESTClient errorHandler(Function, String> errorHandler) { } private Map query(String method, String resource, String body, Map headers) throws IOException { + int attempts = 0; + while (true) { + try { + return queryOnce(method, resource, body, headers); + } catch (SocketTimeoutException e) { + attempts++; + if (attempts >= 3) { + throw e; + } + log.warning("Connection timeout, retrying in 5 seconds (attempt " + attempts + "/3)"); + sleep(5000); + } + } + } + + void sleep(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + + protected HttpURLConnection openConnection(URL url) throws IOException { + return (HttpURLConnection) url.openConnection(); + } + + private Map queryOnce(String method, String resource, String body, Map headers) throws IOException { URL url = new URL(resource.startsWith("http") ? resource : endpoint + resource); log.finest(method + " " + url); - HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + HttpURLConnection conn = openConnection(url); + conn.setConnectTimeout(30000); conn.setRequestMethod(method); String userAgent = System.getProperty("http.agent"); conn.setRequestProperty("User-Agent", "Jsign (https://ebourg.github.io/jsign/)" + (userAgent != null ? " " + userAgent : "")); diff --git a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java new file mode 100644 index 00000000..8c2f500a --- /dev/null +++ b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java @@ -0,0 +1,115 @@ +/* + * Copyright 2024 Emmanuel Bourg + * + * 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 net.jsign.jca; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.SocketTimeoutException; +import java.net.URL; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static net.jadler.Jadler.*; +import static org.junit.Assert.*; + +public class RESTClientTest { + + @Before + public void setUp() { + initJadler().withDefaultResponseStatus(404); + } + + @After + public void tearDown() { + closeJadler(); + } + + @Test + public void testRetryOnTimeout() throws Exception { + AtomicInteger attempts = new AtomicInteger(0); + RESTClient client = new RESTClient("http://localhost:" + port()) { + @Override + protected HttpURLConnection openConnection(URL url) throws IOException { + attempts.incrementAndGet(); + throw new SocketTimeoutException("timeout"); + } + + @Override + void sleep(long millis) { + // don't sleep in tests + } + }; + + assertThrows(SocketTimeoutException.class, () -> client.get("/test")); + assertEquals("attempts", 3, attempts.get()); + } + + @Test + public void testRetryEventuallySucceeds() throws Exception { + AtomicInteger attempts = new AtomicInteger(0); + + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/test") + .respond() + .withStatus(200) + .withBody("{\"status\":\"ok\"}"); + + RESTClient client = new RESTClient("http://localhost:" + port()) { + @Override + protected HttpURLConnection openConnection(URL url) throws IOException { + if (attempts.incrementAndGet() < 3) { + throw new SocketTimeoutException("timeout"); + } + return super.openConnection(url); + } + + @Override + void sleep(long millis) { + // don't sleep in tests + } + }; + + Map response = client.get("/test"); + assertEquals("attempts", 3, attempts.get()); + assertEquals("ok", response.get("status")); + } + + @Test + public void testNoRetryOnOtherException() throws Exception { + AtomicInteger attempts = new AtomicInteger(0); + RESTClient client = new RESTClient("http://localhost:" + port()) { + @Override + protected HttpURLConnection openConnection(URL url) throws IOException { + attempts.incrementAndGet(); + throw new IOException("error"); + } + + @Override + void sleep(long millis) { + // don't sleep in tests + } + }; + + assertThrows(IOException.class, () -> client.get("/test")); + assertEquals("attempts", 1, attempts.get()); + } +} From dc450913b33102b5195eee571dffe6b07b0f5f87 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 16:51:40 +0000 Subject: [PATCH 2/4] Implement configurable HTTP retry logic and timeouts in RESTClient - Implemented retry logic for SocketTimeoutException in RESTClient. - Made retry count, retry pause, connect timeout, and read timeout configurable via fluent setter methods. - Defaulted to 3 attempts, 5s pause, and 30s timeouts. - Added RESTClientTest using WireMock to verify retry behavior on timeout and exhaustion. - Added wiremock-standalone dependency for testing. Co-authored-by: ebourg <54304+ebourg@users.noreply.github.com> --- jsign-crypto/pom.xml | 7 ++ .../main/java/net/jsign/jca/RESTClient.java | 81 ++++++++++--- .../java/net/jsign/jca/RESTClientTest.java | 107 ++++++++---------- 3 files changed, 117 insertions(+), 78 deletions(-) diff --git a/jsign-crypto/pom.xml b/jsign-crypto/pom.xml index 9d8d1f16..2c9ca193 100644 --- a/jsign-crypto/pom.xml +++ b/jsign-crypto/pom.xml @@ -53,6 +53,13 @@ 1.3.1 test + + + org.wiremock + wiremock-standalone + 3.12.0 + test + diff --git a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java index 5af95b83..ace25112 100644 --- a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java +++ b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java @@ -48,6 +48,18 @@ class RESTClient { /** Callback building an error message from the JSON formatted error response */ private Function, String> errorHandler; + /** Connect timeout (in milliseconds) */ + private int connectTimeout = 30000; + + /** Read timeout (in milliseconds) */ + private int readTimeout = 30000; + + /** Number of retries */ + private int retries = 3; + + /** Pause between retries (in milliseconds) */ + private int retryPause = 5000; + public RESTClient(String endpoint) { this.endpoint = endpoint; } @@ -67,6 +79,46 @@ public RESTClient errorHandler(Function, String> errorHandler) { return this; } + /** + * Sets the connect timeout. + * + * @param connectTimeout the timeout in milliseconds + */ + public RESTClient connectTimeout(int connectTimeout) { + this.connectTimeout = connectTimeout; + return this; + } + + /** + * Sets the read timeout. + * + * @param readTimeout the timeout in milliseconds + */ + public RESTClient readTimeout(int readTimeout) { + this.readTimeout = readTimeout; + return this; + } + + /** + * Sets the number of retries. + * + * @param retries the number of retries + */ + public RESTClient retries(int retries) { + this.retries = retries; + return this; + } + + /** + * Sets the pause between retries. + * + * @param retryPause the pause in milliseconds + */ + public RESTClient retryPause(int retryPause) { + this.retryPause = retryPause; + return this; + } + public Map get(String resource) throws IOException { return query("GET", resource, null, null); } @@ -131,32 +183,25 @@ public RESTClient errorHandler(Function, String> errorHandler) { return queryOnce(method, resource, body, headers); } catch (SocketTimeoutException e) { attempts++; - if (attempts >= 3) { + if (attempts >= retries) { throw e; } - log.warning("Connection timeout, retrying in 5 seconds (attempt " + attempts + "/3)"); - sleep(5000); + log.warning("Connection timeout, retrying in " + (retryPause / 1000) + " seconds (attempt " + attempts + "/" + retries + ")"); + try { + Thread.sleep(retryPause); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } } } } - void sleep(long millis) { - try { - Thread.sleep(millis); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - } - } - - protected HttpURLConnection openConnection(URL url) throws IOException { - return (HttpURLConnection) url.openConnection(); - } - - private Map queryOnce(String method, String resource, String body, Map headers) throws IOException { + protected Map queryOnce(String method, String resource, String body, Map headers) throws IOException { URL url = new URL(resource.startsWith("http") ? resource : endpoint + resource); log.finest(method + " " + url); - HttpURLConnection conn = openConnection(url); - conn.setConnectTimeout(30000); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setConnectTimeout(connectTimeout); + conn.setReadTimeout(readTimeout); conn.setRequestMethod(method); String userAgent = System.getProperty("http.agent"); conn.setRequestProperty("User-Agent", "Jsign (https://ebourg.github.io/jsign/)" + (userAgent != null ? " " + userAgent : "")); diff --git a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java index 8c2f500a..87f3e8a0 100644 --- a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java +++ b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java @@ -17,99 +17,86 @@ package net.jsign.jca; import java.io.IOException; -import java.net.HttpURLConnection; import java.net.SocketTimeoutException; -import java.net.URL; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; +import com.github.tomakehurst.wiremock.WireMockServer; import org.junit.After; import org.junit.Before; import org.junit.Test; -import static net.jadler.Jadler.*; +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.junit.Assert.*; public class RESTClientTest { + private WireMockServer wireMockServer; + @Before public void setUp() { - initJadler().withDefaultResponseStatus(404); + wireMockServer = new WireMockServer(wireMockConfig().dynamicPort()); + wireMockServer.start(); } @After public void tearDown() { - closeJadler(); + wireMockServer.stop(); } @Test - public void testRetryOnTimeout() throws Exception { - AtomicInteger attempts = new AtomicInteger(0); - RESTClient client = new RESTClient("http://localhost:" + port()) { - @Override - protected HttpURLConnection openConnection(URL url) throws IOException { - attempts.incrementAndGet(); - throw new SocketTimeoutException("timeout"); - } - - @Override - void sleep(long millis) { - // don't sleep in tests - } - }; + public void testRetryOnTimeout() { + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse() + .withStatus(200) + .withFixedDelay(1000))); + + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) + .readTimeout(100) + .retries(3) + .retryPause(10); assertThrows(SocketTimeoutException.class, () -> client.get("/test")); - assertEquals("attempts", 3, attempts.get()); + wireMockServer.verify(3, getRequestedFor(urlEqualTo("/test"))); } @Test public void testRetryEventuallySucceeds() throws Exception { - AtomicInteger attempts = new AtomicInteger(0); - - onRequest() - .havingMethodEqualTo("GET") - .havingPathEqualTo("/test") - .respond() - .withStatus(200) - .withBody("{\"status\":\"ok\"}"); - - RESTClient client = new RESTClient("http://localhost:" + port()) { - @Override - protected HttpURLConnection openConnection(URL url) throws IOException { - if (attempts.incrementAndGet() < 3) { - throw new SocketTimeoutException("timeout"); - } - return super.openConnection(url); - } - - @Override - void sleep(long millis) { - // don't sleep in tests - } - }; + wireMockServer.stubFor(get(urlEqualTo("/test")).inScenario("Retry Scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(200) + .withFixedDelay(500)) + .willSetStateTo("Succeeded")); + + wireMockServer.stubFor(get(urlEqualTo("/test")).inScenario("Retry Scenario") + .whenScenarioStateIs("Succeeded") + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"status\":\"ok\"}"))); + + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) + .readTimeout(200) + .retries(3) + .retryPause(400); Map response = client.get("/test"); - assertEquals("attempts", 3, attempts.get()); assertEquals("ok", response.get("status")); + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); } @Test - public void testNoRetryOnOtherException() throws Exception { - AtomicInteger attempts = new AtomicInteger(0); - RESTClient client = new RESTClient("http://localhost:" + port()) { - @Override - protected HttpURLConnection openConnection(URL url) throws IOException { - attempts.incrementAndGet(); - throw new IOException("error"); - } - - @Override - void sleep(long millis) { - // don't sleep in tests - } - }; + public void testNoRetryOnOtherException() { + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse() + .withStatus(404))); + + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) + .retries(3) + .retryPause(10); assertThrows(IOException.class, () -> client.get("/test")); - assertEquals("attempts", 1, attempts.get()); + wireMockServer.verify(1, getRequestedFor(urlEqualTo("/test"))); } } From b84597919cab478627e8d47efc723782369732bb Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 18:08:41 +0000 Subject: [PATCH 3/4] Implement configurable HTTP retry logic and timeouts in RESTClient - Implemented retry logic for SocketTimeoutException in RESTClient. - Made retry count, retry pause, connect timeout, and read timeout configurable via plain setter methods. - Defaulted to 3 attempts, 5s pause, and 30s timeouts. - Added RESTClientTest using WireMock to verify retry behavior on timeout and exhaustion. - Added wiremock-standalone dependency for testing. Co-authored-by: ebourg <54304+ebourg@users.noreply.github.com> --- .../main/java/net/jsign/jca/RESTClient.java | 14 +++++------- .../java/net/jsign/jca/RESTClientTest.java | 22 +++++++++---------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java index ace25112..1ed48ad9 100644 --- a/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java +++ b/jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java @@ -84,9 +84,8 @@ public RESTClient errorHandler(Function, String> errorHandler) { * * @param connectTimeout the timeout in milliseconds */ - public RESTClient connectTimeout(int connectTimeout) { + public void setConnectTimeout(int connectTimeout) { this.connectTimeout = connectTimeout; - return this; } /** @@ -94,9 +93,8 @@ public RESTClient connectTimeout(int connectTimeout) { * * @param readTimeout the timeout in milliseconds */ - public RESTClient readTimeout(int readTimeout) { + public void setReadTimeout(int readTimeout) { this.readTimeout = readTimeout; - return this; } /** @@ -104,9 +102,8 @@ public RESTClient readTimeout(int readTimeout) { * * @param retries the number of retries */ - public RESTClient retries(int retries) { + public void setRetries(int retries) { this.retries = retries; - return this; } /** @@ -114,9 +111,8 @@ public RESTClient retries(int retries) { * * @param retryPause the pause in milliseconds */ - public RESTClient retryPause(int retryPause) { + public void setRetryPause(int retryPause) { this.retryPause = retryPause; - return this; } public Map get(String resource) throws IOException { @@ -196,7 +192,7 @@ public RESTClient retryPause(int retryPause) { } } - protected Map queryOnce(String method, String resource, String body, Map headers) throws IOException { + private Map queryOnce(String method, String resource, String body, Map headers) throws IOException { URL url = new URL(resource.startsWith("http") ? resource : endpoint + resource); log.finest(method + " " + url); HttpURLConnection conn = (HttpURLConnection) url.openConnection(); diff --git a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java index 87f3e8a0..8bd4f2d9 100644 --- a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java +++ b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java @@ -51,10 +51,10 @@ public void testRetryOnTimeout() { .withStatus(200) .withFixedDelay(1000))); - RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) - .readTimeout(100) - .retries(3) - .retryPause(10); + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()); + client.setReadTimeout(100); + client.setRetries(3); + client.setRetryPause(10); assertThrows(SocketTimeoutException.class, () -> client.get("/test")); wireMockServer.verify(3, getRequestedFor(urlEqualTo("/test"))); @@ -76,10 +76,10 @@ public void testRetryEventuallySucceeds() throws Exception { .withHeader("Content-Type", "application/json") .withBody("{\"status\":\"ok\"}"))); - RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) - .readTimeout(200) - .retries(3) - .retryPause(400); + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()); + client.setReadTimeout(200); + client.setRetries(3); + client.setRetryPause(400); Map response = client.get("/test"); assertEquals("ok", response.get("status")); @@ -92,9 +92,9 @@ public void testNoRetryOnOtherException() { .willReturn(aResponse() .withStatus(404))); - RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()) - .retries(3) - .retryPause(10); + RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()); + client.setRetries(3); + client.setRetryPause(10); assertThrows(IOException.class, () -> client.get("/test")); wireMockServer.verify(1, getRequestedFor(urlEqualTo("/test"))); From 86676800b908e221d48a1d53f79040fac144d4ef Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 9 Mar 2026 18:15:45 +0000 Subject: [PATCH 4/4] Implement configurable HTTP retry logic and timeouts in RESTClient - Implemented retry logic for SocketTimeoutException in RESTClient. - Made retry count, retry pause, connect timeout, and read timeout configurable via plain setter methods. - Defaulted to 3 attempts, 5s pause, and 30s timeouts. - Added RESTClientTest using WireMock to verify retry behavior on timeout and recovery. - Added wiremock-standalone 2.27.2 dependency for Java 8 compatibility. Co-authored-by: ebourg <54304+ebourg@users.noreply.github.com> --- jsign-crypto/pom.xml | 4 ++-- jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jsign-crypto/pom.xml b/jsign-crypto/pom.xml index 2c9ca193..5480e307 100644 --- a/jsign-crypto/pom.xml +++ b/jsign-crypto/pom.xml @@ -55,9 +55,9 @@ - org.wiremock + com.github.tomakehurst wiremock-standalone - 3.12.0 + 2.27.2 test diff --git a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java index 8bd4f2d9..3fa23e75 100644 --- a/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java +++ b/jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java @@ -78,7 +78,7 @@ public void testRetryEventuallySucceeds() throws Exception { RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port()); client.setReadTimeout(200); - client.setRetries(3); + client.setRetries(4); // allow enough retries client.setRetryPause(400); Map response = client.get("/test");