From 6d5da63b8ecff56d2e68b068f91ad9f4544a9435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Wed, 15 May 2024 15:39:56 +0200 Subject: [PATCH] Handle HTTP errors when fetching a nonce The nonce is fetched via HEAD request. Before this fix, if there was a HTTP error, acme4j expected a Problem JSON body, which was not send because of the HEAD request, and lead to an AcmeProtocolException. Now either an AcmeException or AcmeRetryAfterException is thrown. --- .../shredzone/acme4j/AcmeJsonResource.java | 4 ++ .../acme4j/connector/DefaultConnection.java | 8 ++- .../connector/DefaultConnectionTest.java | 52 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java index 59447b69..82df6fea 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/AcmeJsonResource.java @@ -152,6 +152,10 @@ public abstract class AcmeJsonResource extends AcmeResource { retryAfterOpt.ifPresent(instant -> LOG.debug("Retry-After: {}", instant)); setRetryAfter(retryAfterOpt.orElse(null)); return retryAfterOpt; + } catch (AcmeRetryAfterException ex) { + LOG.debug("Retry-After while attempting to read the resource", ex); + setRetryAfter(ex.getRetryAfter()); + return Optional.of(ex.getRetryAfter()); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java index 8e4fdcdc..aad0869b 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/DefaultConnection.java @@ -51,6 +51,7 @@ import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeNetworkException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRateLimitedException; +import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.exception.AcmeServerException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; import org.shredzone.acme4j.exception.AcmeUserActionRequiredException; @@ -132,7 +133,12 @@ public class DefaultConnection implements Connection { var rc = getResponse().statusCode(); if (rc != HTTP_OK && rc != HTTP_NO_CONTENT) { - throwAcmeException(); + var message = "Server responded with HTTP " + rc + " while trying to retrieve a nonce"; + var retryAfterInstant = getRetryAfter(); + if (retryAfterInstant.isPresent()) { + throw new AcmeRetryAfterException(message, retryAfterInstant.get()); + }; + throw new AcmeException(message); } session.setNonce(getNonce() diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java index 57659158..fbaf8f6e 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DefaultConnectionTest.java @@ -52,6 +52,7 @@ import org.shredzone.acme4j.Session; import org.shredzone.acme4j.exception.AcmeException; import org.shredzone.acme4j.exception.AcmeProtocolException; import org.shredzone.acme4j.exception.AcmeRateLimitedException; +import org.shredzone.acme4j.exception.AcmeRetryAfterException; import org.shredzone.acme4j.exception.AcmeServerException; import org.shredzone.acme4j.exception.AcmeUnauthorizedException; import org.shredzone.acme4j.exception.AcmeUserActionRequiredException; @@ -142,6 +143,57 @@ public class DefaultConnectionTest { verify(getRequestedFor(urlEqualTo(REQUEST_PATH))); } + /** + * Test that {@link DefaultConnection#getNonce()} handles a retry-after header + * correctly. + */ + @Test + public void testGetNonceFromHeaderRetryAfter() { + var retryAfter = Instant.now().plusSeconds(30L).truncatedTo(SECONDS); + + stubFor(head(urlEqualTo(NEW_NONCE_PATH)).willReturn(aResponse() + .withStatus(HttpURLConnection.HTTP_UNAVAILABLE) + .withHeader("Content-Type", "application/problem+json") + .withHeader("Retry-After", DATE_FORMATTER.format(retryAfter)) + // do not send a body here because it is a HEAD request! + )); + + assertThat(session.getNonce()).isNull(); + + var ex = assertThrows(AcmeRetryAfterException.class, () -> { + try (var conn = session.connect()) { + conn.resetNonce(session); + } + }); + assertThat(ex.getMessage()).isEqualTo("Server responded with HTTP 503 while trying to retrieve a nonce"); + assertThat(ex.getRetryAfter()).isEqualTo(retryAfter); + + verify(headRequestedFor(urlEqualTo(NEW_NONCE_PATH))); + } + + /** + * Test that {@link DefaultConnection#getNonce()} handles a general HTTP error + * correctly. + */ + @Test + public void testGetNonceFromHeaderHttpError() { + stubFor(head(urlEqualTo(NEW_NONCE_PATH)).willReturn(aResponse() + .withStatus(HttpURLConnection.HTTP_INTERNAL_ERROR) + // do not send a body here because it is a HEAD request! + )); + + assertThat(session.getNonce()).isNull(); + + var ex = assertThrows(AcmeException.class, () -> { + try (var conn = session.connect()) { + conn.resetNonce(session); + } + }); + assertThat(ex.getMessage()).isEqualTo("Server responded with HTTP 500 while trying to retrieve a nonce"); + + verify(headRequestedFor(urlEqualTo(NEW_NONCE_PATH))); + } + /** * Test that {@link DefaultConnection#getNonce()} fails on an invalid * {@code Replay-Nonce} header.