diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java index 865c9ebc..7387be88 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Session.java @@ -17,8 +17,7 @@ import java.net.Proxy; import java.net.URI; import java.net.URL; import java.security.KeyPair; -import java.time.Duration; -import java.time.Instant; +import java.time.ZonedDateTime; import java.util.EnumMap; import java.util.Locale; import java.util.Map; @@ -58,7 +57,8 @@ public class Session { private String nonce; private Locale locale = Locale.getDefault(); - protected Instant directoryCacheExpiry; + protected ZonedDateTime directoryLastModified; + protected ZonedDateTime directoryExpires; /** * Creates a new {@link Session}. @@ -250,19 +250,80 @@ public class Session { } /** - * Reads the provider's directory, then rebuild the resource map. The response is - * cached. + * Returns the date when the directory has been modified the last time. + * + * @return The last modification date of the directory, or {@code null} if unknown + * (directory has not been read yet or did not provide this information). + * @since 2.10 + */ + @CheckForNull + public ZonedDateTime getDirectoryLastModified() { + return directoryLastModified; + } + + /** + * Sets the date when the directory has been modified the last time. Should only be + * invoked by {@link AcmeProvider} implementations. + * + * @param directoryLastModified + * The last modification date of the directory, or {@code null} if unknown + * (directory has not been read yet or did not provide this information). + * @since 2.10 + */ + public void setDirectoryLastModified(@Nullable ZonedDateTime directoryLastModified) { + this.directoryLastModified = directoryLastModified; + } + + /** + * Returns the date when the current directory records will expire. A fresh copy of + * the directory will be fetched automatically after that instant. + * + * @return The expiration date, or {@code null} if the server did not provide this + * information. + * @since 2.10 + */ + @CheckForNull + public ZonedDateTime getDirectoryExpires() { + return directoryExpires; + } + + /** + * Sets the date when the current directory will expire. Should only be invoked by + * {@link AcmeProvider} implementations. + * + * @param directoryExpires + * Expiration date, or {@code null} if the server did not provide this + * information. + * @since 2.10 + */ + public void setDirectoryExpires(@Nullable ZonedDateTime directoryExpires) { + this.directoryExpires = directoryExpires; + } + + /** + * Returns {@code true} if a directory is available. Should only be invoked by {@link + * AcmeProvider} implementations. + * + * @return {@code true} if a directory is available. + * @since 2.10 + */ + public boolean hasDirectory() { + return resourceMap.get() != null; + } + + /** + * Reads the provider's directory, then rebuild the resource map. The resource map + * is unchanged if the {@link AcmeProvider} returns that the directory has not been + * changed on the remote side. */ private void readDirectory() throws AcmeException { - synchronized (this) { - Instant now = Instant.now(); - if (directoryCacheExpiry != null && directoryCacheExpiry.isAfter(now)) { - return; - } - directoryCacheExpiry = now.plus(Duration.ofHours(1)); - } - JSON directoryJson = provider().directory(this, getServerUri()); + if (directoryJson == null) { + if (!hasDirectory()) { + throw new AcmeException("AcmeProvider did not provide a directory"); + } + return; + } Value meta = directoryJson.get("meta"); if (meta.isPresent()) { diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java index d905f0f2..15504c5e 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/Connection.java @@ -17,10 +17,13 @@ import java.net.HttpURLConnection; import java.net.URL; import java.security.KeyPair; import java.security.cert.X509Certificate; +import java.time.ZonedDateTime; import java.util.Collection; import java.util.List; +import java.util.Optional; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import javax.annotation.ParametersAreNonnullByDefault; import org.shredzone.acme4j.Login; @@ -55,8 +58,13 @@ public interface Connection extends AutoCloseable { * {@link URL} to send the request to. * @param session * {@link Session} instance to be used for tracking + * @param ifModifiedSince + * {@link ZonedDateTime} to be sent as "If-Modified-Since" header, or + * {@code null} if this header is not to be used + * @return HTTP status that was returned */ - void sendRequest(URL url, Session session) throws AcmeException; + int sendRequest(URL url, Session session, @Nullable ZonedDateTime ifModifiedSince) + throws AcmeException; /** * Sends a signed POST-as-GET request for a certificate resource. Requires a @@ -171,6 +179,24 @@ public interface Connection extends AutoCloseable { @CheckForNull URL getLocation(); + /** + * Returns the content of the last-modified header, if present. + * + * @return Date in the Last-Modified header, or empty if the server did not provide + * this information. + * @since 2.10 + */ + Optional getLastModified(); + + /** + * Returns the expiration date of the resource, if present. + * + * @return Expiration date, either from the Cache-Control or Expires header. If empty, + * the server did not provide an expiration date, or forbid caching. + * @since 2.10 + */ + Optional getExpiration(); + /** * Gets one or more relation links from the header. The result is expected to be an URL. *

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 df74ca10..2413daad 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 @@ -13,6 +13,7 @@ */ package org.shredzone.acme4j.connector; +import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME; import static java.util.stream.Collectors.toList; import java.io.IOException; @@ -29,6 +30,9 @@ import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.time.format.DateTimeParseException; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -69,8 +73,12 @@ public class DefaultConnection implements Connection { private static final String ACCEPT_HEADER = "Accept"; private static final String ACCEPT_CHARSET_HEADER = "Accept-Charset"; private static final String ACCEPT_LANGUAGE_HEADER = "Accept-Language"; + private static final String CACHE_CONTROL_HEADER = "Cache-Control"; private static final String CONTENT_TYPE_HEADER = "Content-Type"; private static final String DATE_HEADER = "Date"; + private static final String EXPIRES_HEADER = "Expires"; + private static final String IF_MODIFIED_SINCE_HEADER = "If-Modified-Since"; + private static final String LAST_MODIFIED_HEADER = "Last-Modified"; private static final String LINK_HEADER = "Link"; private static final String LOCATION_HEADER = "Location"; private static final String REPLAY_NONCE_HEADER = "Replay-Nonce"; @@ -83,6 +91,9 @@ public class DefaultConnection implements Connection { private static final URI BAD_NONCE_ERROR = URI.create("urn:ietf:params:acme:error:badNonce"); private static final int MAX_ATTEMPTS = 10; + private static final Pattern NO_CACHE_PATTERN = Pattern.compile("(?:^|.*?,)\\s*no-(?:cache|store)\\s*(?:,.*|$)", Pattern.CASE_INSENSITIVE); + private static final Pattern MAX_AGE_PATTERN = Pattern.compile("(?:^|.*?,)\\s*max-age=(\\d+)\\s*(?:,.*|$)", Pattern.CASE_INSENSITIVE); + protected final HttpConnector httpConnector; protected HttpURLConnection conn; @@ -132,8 +143,9 @@ public class DefaultConnection implements Connection { } @Override - public void sendRequest(URL url, Session session) throws AcmeException { - sendRequest(url, session, MIME_JSON); + public int sendRequest(URL url, Session session, @Nullable ZonedDateTime ifModifiedSince) + throws AcmeException { + return sendRequest(url, session, MIME_JSON, ifModifiedSince); } @Override @@ -252,6 +264,54 @@ public class DefaultConnection implements Connection { return resolveRelative(location); } + @Override + public Optional getLastModified() { + assertConnectionIsOpen(); + + String header = conn.getHeaderField(LAST_MODIFIED_HEADER); + if (header != null) { + try { + return Optional.of(ZonedDateTime.parse(header, RFC_1123_DATE_TIME)); + } catch (DateTimeParseException ex) { + LOG.debug("Ignored invalid Last-Modified date: {}", header, ex); + } + } + return Optional.empty(); + } + + @Override + public Optional getExpiration() { + assertConnectionIsOpen(); + + String cacheHeader = conn.getHeaderField(CACHE_CONTROL_HEADER); + if (cacheHeader != null) { + if (NO_CACHE_PATTERN.matcher(cacheHeader).matches()) { + return Optional.empty(); + } + + Matcher m = MAX_AGE_PATTERN.matcher(cacheHeader); + if (m.matches()) { + int maxAge = Integer.parseInt(m.group(1)); + if (maxAge == 0) { + return Optional.empty(); + } + + return Optional.of(ZonedDateTime.now(ZoneId.of("UTC")).plusSeconds(maxAge)); + } + } + + String expiresHeader = conn.getHeaderField(EXPIRES_HEADER); + if (expiresHeader != null) { + try { + return Optional.of(ZonedDateTime.parse(expiresHeader, RFC_1123_DATE_TIME)); + } catch (DateTimeParseException ex) { + LOG.debug("Ignored invalid Expires date: {}", expiresHeader, ex); + } + } + + return Optional.empty(); + } + @Override public Collection getLinks(String relation) { return collectLinks(relation).stream() @@ -273,9 +333,13 @@ public class DefaultConnection implements Connection { * {@link Session} instance to be used for signing and tracking * @param accept * Accept header + * @param ifModifiedSince + * Set an If-Modified-Since header with the given date. If set, an + * NOT_MODIFIED response is accepted as valid. * @return HTTP 200 class status that was returned */ - protected int sendRequest(URL url, Session session, String accept) throws AcmeException { + protected int sendRequest(URL url, Session session, String accept, + @Nullable ZonedDateTime ifModifiedSince) throws AcmeException { Objects.requireNonNull(url, "url"); Objects.requireNonNull(session, "session"); Objects.requireNonNull(accept, "accept"); @@ -289,6 +353,9 @@ public class DefaultConnection implements Connection { conn.setRequestProperty(ACCEPT_HEADER, accept); conn.setRequestProperty(ACCEPT_CHARSET_HEADER, DEFAULT_CHARSET); conn.setRequestProperty(ACCEPT_LANGUAGE_HEADER, session.getLocale().toLanguageTag()); + if (ifModifiedSince != null) { + conn.setRequestProperty(IF_MODIFIED_SINCE_HEADER, ifModifiedSince.format(RFC_1123_DATE_TIME)); + } conn.setDoOutput(false); conn.connect(); @@ -301,7 +368,8 @@ public class DefaultConnection implements Connection { } int rc = conn.getResponseCode(); - if (rc != HttpURLConnection.HTTP_OK && rc != HttpURLConnection.HTTP_CREATED) { + if (rc != HttpURLConnection.HTTP_OK && rc != HttpURLConnection.HTTP_CREATED + && (rc != HttpURLConnection.HTTP_NOT_MODIFIED || ifModifiedSince == null)) { throwAcmeException(); } return rc; diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java index 1022c887..6cf9b629 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AbstractAcmeProvider.java @@ -13,7 +13,9 @@ */ package org.shredzone.acme4j.provider; +import java.net.HttpURLConnection; import java.net.URI; +import java.time.ZonedDateTime; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -54,8 +56,23 @@ public abstract class AbstractAcmeProvider implements AcmeProvider { @Override public JSON directory(Session session, URI serverUri) throws AcmeException { + ZonedDateTime expires = session.getDirectoryExpires(); + if (expires != null && expires.isAfter(ZonedDateTime.now())) { + // The cached directory is still valid + return null; + } + try (Connection conn = connect(serverUri)) { - conn.sendRequest(resolve(serverUri), session); + ZonedDateTime lastModified = session.getDirectoryLastModified(); + int rc = conn.sendRequest(resolve(serverUri), session, lastModified); + if (lastModified != null && rc == HttpURLConnection.HTTP_NOT_MODIFIED) { + // The server has not been modified since + return null; + } + + // evaluate caching headers + session.setDirectoryLastModified(conn.getLastModified().orElse(null)); + session.setDirectoryExpires(conn.getExpiration().orElse(null)); // use nonce header if there is one, saves a HEAD request... String nonce = conn.getNonce(); diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeProvider.java b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeProvider.java index 87177ec5..a50b26b7 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeProvider.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/provider/AcmeProvider.java @@ -77,8 +77,10 @@ public interface AcmeProvider { * {@link Session} to be used * @param serverUri * Server {@link URI} - * @return Directory data, as JSON object + * @return Directory data, as JSON object, or {@code null} if the directory has not + * been changed since the last request. */ + @CheckForNull JSON directory(Session session, URI serverUri) throws AcmeException; /** diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java index aa3945d2..8b6d9435 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/SessionTest.java @@ -24,7 +24,7 @@ import java.net.URI; import java.net.URL; import java.security.KeyPair; import java.time.Duration; -import java.time.Instant; +import java.time.ZonedDateTime; import org.junit.Test; import org.mockito.ArgumentMatchers; @@ -86,6 +86,7 @@ public class SessionTest { @Test public void testGettersAndSetters() { URI serverUri = URI.create(TestUtils.ACME_SERVER_URI); + ZonedDateTime now = ZonedDateTime.now(); Session session = new Session(serverUri); @@ -95,6 +96,18 @@ public class SessionTest { assertThat(session.getServerUri(), is(serverUri)); assertThat(session.networkSettings(), is(notNullValue())); + + assertThat(session.getDirectoryExpires(), is(nullValue())); + session.setDirectoryExpires(now); + assertThat(session.getDirectoryExpires(), is(equalTo(now))); + session.setDirectoryExpires(null); + assertThat(session.getDirectoryExpires(), is(nullValue())); + + assertThat(session.getDirectoryLastModified(), is(nullValue())); + session.setDirectoryLastModified(now); + assertThat(session.getDirectoryLastModified(), is(equalTo(now))); + session.setDirectoryLastModified(null); + assertThat(session.getDirectoryLastModified(), is(nullValue())); } /** @@ -116,7 +129,7 @@ public class SessionTest { } /** - * Test that the directory is properly read and cached. + * Test that the directory is properly read. */ @Test public void testDirectory() throws AcmeException, IOException { @@ -135,19 +148,41 @@ public class SessionTest { } }; - assertSession(session); + // No directory has been fetched yet + assertThat(session.hasDirectory(), is(false)); - // Make sure directory is only read once! - verify(mockProvider, times(1)).directory( - ArgumentMatchers.any(Session.class), - ArgumentMatchers.any(URI.class)); + assertThat(session.resourceUrl(Resource.NEW_ACCOUNT), + is(new URL("https://example.com/acme/new-account"))); - // Simulate a cache expiry - session.directoryCacheExpiry = Instant.now(); + // There is a local copy of the directory now + assertThat(session.hasDirectory(), is(true)); - // Make sure directory is read once again - assertSession(session); - verify(mockProvider, times(2)).directory( + assertThat(session.resourceUrl(Resource.NEW_AUTHZ), + is(new URL("https://example.com/acme/new-authz"))); + assertThat(session.resourceUrl(Resource.NEW_ORDER), + is(new URL("https://example.com/acme/new-order"))); + + try { + session.resourceUrl(Resource.REVOKE_CERT); + fail("Did not fail to get an unsupported resource URL"); + } catch (AcmeException ex) { + // Expected + } + + Metadata meta = session.getMetadata(); + assertThat(meta, not(nullValue())); + assertThat(meta.getTermsOfService(), is(URI.create("https://example.com/acme/terms"))); + assertThat(meta.getWebsite(), is(url("https://www.example.com/"))); + assertThat(meta.getCaaIdentities(), containsInAnyOrder("example.com")); + assertThat(meta.isAutoRenewalEnabled(), is(true)); + assertThat(meta.getAutoRenewalMaxDuration(), is(Duration.ofDays(365))); + assertThat(meta.getAutoRenewalMinLifetime(), is(Duration.ofHours(24))); + assertThat(meta.isAutoRenewalGetAllowed(), is(true)); + assertThat(meta.isExternalAccountRequired(), is(true)); + assertThat(meta.getJSON(), is(notNullValue())); + + // Make sure directory is read + verify(mockProvider, atLeastOnce()).directory( ArgumentMatchers.any(Session.class), ArgumentMatchers.any(URI.class)); } @@ -190,39 +225,4 @@ public class SessionTest { assertThat(meta.isAutoRenewalGetAllowed(), is(false)); } - /** - * Asserts that the {@link Session} returns correct - * {@link Session#resourceUrl(Resource)} and {@link Session#getMetadata()}. - * - * @param session - * {@link Session} to assert - */ - private void assertSession(Session session) throws AcmeException, IOException { - assertThat(session.resourceUrl(Resource.NEW_ACCOUNT), - is(new URL("https://example.com/acme/new-account"))); - assertThat(session.resourceUrl(Resource.NEW_AUTHZ), - is(new URL("https://example.com/acme/new-authz"))); - assertThat(session.resourceUrl(Resource.NEW_ORDER), - is(new URL("https://example.com/acme/new-order"))); - - try { - session.resourceUrl(Resource.REVOKE_CERT); - fail("Did not fail to get an unsupported resource URL"); - } catch (AcmeException ex) { - // Expected - } - - Metadata meta = session.getMetadata(); - assertThat(meta, not(nullValue())); - assertThat(meta.getTermsOfService(), is(URI.create("https://example.com/acme/terms"))); - assertThat(meta.getWebsite(), is(url("https://www.example.com/"))); - assertThat(meta.getCaaIdentities(), containsInAnyOrder("example.com")); - assertThat(meta.isAutoRenewalEnabled(), is(true)); - assertThat(meta.getAutoRenewalMaxDuration(), is(Duration.ofDays(365))); - assertThat(meta.getAutoRenewalMinLifetime(), is(Duration.ofHours(24))); - assertThat(meta.isAutoRenewalGetAllowed(), is(true)); - assertThat(meta.isExternalAccountRequired(), is(true)); - assertThat(meta.getJSON(), is(notNullValue())); - } - } 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 0f609ba2..0ee99c31 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 @@ -14,6 +14,7 @@ package org.shredzone.acme4j.connector; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME; import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; @@ -33,6 +34,9 @@ import java.security.KeyPair; import java.security.cert.X509Certificate; import java.time.Duration; import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Base64; @@ -41,6 +45,7 @@ import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Optional; import org.jose4j.jws.JsonWebSignature; import org.jose4j.jwx.CompactSerializer; @@ -660,7 +665,7 @@ public class DefaultConnectionTest { return null; } }) { - conn.sendRequest(requestUrl, session); + conn.sendRequest(requestUrl, session, null); } verify(mockUrlConnection).setRequestMethod("GET"); @@ -674,6 +679,37 @@ public class DefaultConnectionTest { verifyNoMoreInteractions(mockUrlConnection); } + /** + * Test GET requests with If-Modified-Since. + */ + @Test + public void testSendRequestIfModifiedSince() throws Exception { + ZonedDateTime ifModifiedSince = ZonedDateTime.now(ZoneId.of("UTC")); + + when(mockUrlConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_NOT_MODIFIED); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection) { + @Override + public String getNonce() { + return null; + } + }) { + int rc = conn.sendRequest(requestUrl, session, ifModifiedSince); + assertThat(rc, is(HttpURLConnection.HTTP_NOT_MODIFIED)); + } + + verify(mockUrlConnection).setRequestMethod("GET"); + verify(mockUrlConnection).setRequestProperty("Accept", "application/json"); + verify(mockUrlConnection).setRequestProperty("Accept-Charset", "utf-8"); + verify(mockUrlConnection).setRequestProperty("Accept-Language", "ja-JP"); + verify(mockUrlConnection).setRequestProperty("If-Modified-Since", ifModifiedSince.format(RFC_1123_DATE_TIME)); + verify(mockUrlConnection).setDoOutput(false); + verify(mockUrlConnection).connect(); + verify(mockUrlConnection).getResponseCode(); + verify(mockUrlConnection, atLeast(0)).getHeaderFields(); + verifyNoMoreInteractions(mockUrlConnection); + } + /** * Test signed POST requests. */ @@ -1050,4 +1086,159 @@ public class DefaultConnectionTest { } } + /** + * Test that {@link DefaultConnection#getLastModified()} returns valid dates. + */ + @Test + public void testLastModifiedUnset() { + when(mockUrlConnection.getHeaderField("Last-Modified")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getLastModified().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Last-Modified"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testLastModifiedSet() { + when(mockUrlConnection.getHeaderField("Last-Modified")).thenReturn("Thu, 07 May 2020 19:42:46 GMT"); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + Optional lm = conn.getLastModified(); + assertThat(lm.isPresent(), is(true)); + assertThat(lm.get().format(DateTimeFormatter.ISO_DATE_TIME), + is("2020-05-07T19:42:46Z")); + } + + verify(mockUrlConnection).getHeaderField("Last-Modified"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testLastModifiedInvalid() { + when(mockUrlConnection.getHeaderField("Last-Modified")).thenReturn("iNvAlId"); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getLastModified().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Last-Modified"); + verifyNoMoreInteractions(mockUrlConnection); + } + + /** + * Test that {@link DefaultConnection#getExpiration()} returns valid dates. + */ + @Test + public void testExpirationUnset() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn(null); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getExpiration().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verify(mockUrlConnection).getHeaderField("Expires"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationNoCache() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn("public, no-cache"); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getExpiration().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationMaxAgeZero() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn("public, max-age=0, no-cache"); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getExpiration().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationMaxAgeButNoCache() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn("public, max-age=3600, no-cache"); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getExpiration().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationMaxAge() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn("max-age=3600"); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn(null); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + Optional exp = conn.getExpiration(); + assertThat(exp.isPresent(), is(true)); + assertThat(exp.get().isAfter(ZonedDateTime.now().plusHours(1).minusMinutes(1)), is(true)); + assertThat(exp.get().isBefore(ZonedDateTime.now().plusHours(1).plusMinutes(1)), is(true)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationExpires() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn(null); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn("Thu, 18 Jun 2020 08:43:04 GMT"); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + Optional exp = conn.getExpiration(); + assertThat(exp.isPresent(), is(true)); + assertThat(exp.get().format(DateTimeFormatter.ISO_DATE_TIME), + is("2020-06-18T08:43:04Z")); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verify(mockUrlConnection).getHeaderField("Expires"); + verifyNoMoreInteractions(mockUrlConnection); + } + + @Test + public void testExpirationInvalidExpires() { + when(mockUrlConnection.getHeaderField("Cache-Control")).thenReturn(null); + when(mockUrlConnection.getHeaderField("Expires")).thenReturn("iNvAlId"); + + try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { + conn.conn = mockUrlConnection; + assertThat(conn.getExpiration().isPresent(), is(false)); + } + + verify(mockUrlConnection).getHeaderField("Cache-Control"); + verify(mockUrlConnection).getHeaderField("Expires"); + verifyNoMoreInteractions(mockUrlConnection); + } + } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java index 3c3c0294..b6e35c63 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/DummyConnection.java @@ -16,8 +16,10 @@ package org.shredzone.acme4j.connector; import java.net.URL; import java.security.KeyPair; import java.security.cert.X509Certificate; +import java.time.ZonedDateTime; import java.util.Collection; import java.util.List; +import java.util.Optional; import org.shredzone.acme4j.Login; import org.shredzone.acme4j.Session; @@ -37,7 +39,7 @@ public class DummyConnection implements Connection { } @Override - public void sendRequest(URL url, Session session) { + public int sendRequest(URL url, Session session, ZonedDateTime ifModifiedSince) { throw new UnsupportedOperationException(); } @@ -88,6 +90,16 @@ public class DummyConnection implements Connection { throw new UnsupportedOperationException(); } + @Override + public Optional getLastModified() { + throw new UnsupportedOperationException(); + } + + @Override + public Optional getExpiration() { + throw new UnsupportedOperationException(); + } + @Override public Collection getLinks(String relation) { throw new UnsupportedOperationException(); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java index e1a2fece..444143b2 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/provider/AbstractAcmeProviderTest.java @@ -16,14 +16,22 @@ package org.shredzone.acme4j.provider; import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.*; import static org.shredzone.acme4j.toolbox.TestUtils.getJSON; import static uk.co.datumedge.hamcrest.json.SameJSONAs.sameJSONAs; +import java.io.IOException; +import java.net.HttpURLConnection; import java.net.URI; import java.net.URL; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; +import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import javax.annotation.ParametersAreNonnullByDefault; + import org.junit.Test; import org.shredzone.acme4j.Login; import org.shredzone.acme4j.Session; @@ -46,26 +54,17 @@ import org.shredzone.acme4j.toolbox.TestUtils; */ public class AbstractAcmeProviderTest { + private static final URI SERVER_URI = URI.create("http://example.com/acme"); + private static final URL RESOLVED_URL = TestUtils.url("http://example.com/acme/directory"); + /** * Test that connect returns a connection. */ @Test public void testConnect() { - final URI testServerUri = URI.create("http://example.com/acme"); - final AtomicBoolean invoked = new AtomicBoolean(); - AbstractAcmeProvider provider = new AbstractAcmeProvider() { - @Override - public boolean accepts(URI serverUri) { - throw new UnsupportedOperationException(); - } - - @Override - public URL resolve(URI serverUri) { - throw new UnsupportedOperationException(); - } - + AbstractAcmeProvider provider = new TestAbstractAcmeProvider() { @Override protected HttpConnector createHttpConnector() { invoked.set(true); @@ -73,7 +72,7 @@ public class AbstractAcmeProviderTest { } }; - Connection connection = provider.connect(testServerUri); + Connection connection = provider.connect(SERVER_URI); assertThat(connection, not(nullValue())); assertThat(connection, instanceOf(DefaultConnection.class)); assertThat(invoked.get(), is(true)); @@ -84,43 +83,174 @@ public class AbstractAcmeProviderTest { */ @Test public void testResources() throws AcmeException { - final URI testServerUri = URI.create("http://example.com/acme"); - final URL testResolvedUrl = TestUtils.url("http://example.com/acme/directory"); final Connection connection = mock(Connection.class); final Session session = mock(Session.class); when(connection.readJsonResponse()).thenReturn(getJSON("directory")); - AbstractAcmeProvider provider = new AbstractAcmeProvider() { - @Override - public Connection connect(URI serverUri) { - assertThat(serverUri, is(testServerUri)); - return connection; - } + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(connection); + JSON map = provider.directory(session, SERVER_URI); - @Override - public boolean accepts(URI serverUri) { - assertThat(serverUri, is(testServerUri)); - return true; - } - - @Override - public URL resolve(URI serverUri) { - assertThat(serverUri, is(testServerUri)); - return testResolvedUrl; - } - }; - - JSON map = provider.directory(session, testServerUri); assertThat(map.toString(), sameJSONAs(TestUtils.getJSON("directory").toString())); - verify(connection).sendRequest(testResolvedUrl, session); + verify(connection).sendRequest(RESOLVED_URL, session, null); verify(connection).getNonce(); + verify(connection).getLastModified(); + verify(connection).getExpiration(); verify(connection).readJsonResponse(); verify(connection).close(); verifyNoMoreInteractions(connection); } + /** + * Verify that the cache control headers are evaluated. + */ + @Test + public void testResourcesCacheControl() throws AcmeException { + ZonedDateTime lastModified = ZonedDateTime.now().minus(13, ChronoUnit.DAYS); + ZonedDateTime expiryDate = ZonedDateTime.now().plus(60, ChronoUnit.DAYS); + + final Connection connection = mock(Connection.class); + final Session session = mock(Session.class); + + when(connection.readJsonResponse()).thenReturn(getJSON("directory")); + when(connection.getLastModified()).thenReturn(Optional.of(lastModified)); + when(connection.getExpiration()).thenReturn(Optional.of(expiryDate)); + when(session.getDirectoryExpires()).thenReturn(null); + when(session.getDirectoryLastModified()).thenReturn(null); + + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(connection); + JSON map = provider.directory(session, SERVER_URI); + + assertThat(map.toString(), sameJSONAs(TestUtils.getJSON("directory").toString())); + + verify(session).setDirectoryLastModified(eq(lastModified)); + verify(session).setDirectoryExpires(eq(expiryDate)); + verify(session).getDirectoryExpires(); + verify(session).getDirectoryLastModified(); + verifyNoMoreInteractions(session); + + verify(connection).sendRequest(RESOLVED_URL, session, null); + verify(connection).getNonce(); + verify(connection).getLastModified(); + verify(connection).getExpiration(); + verify(connection).readJsonResponse(); + verify(connection).close(); + verifyNoMoreInteractions(connection); + } + + /** + * Verify that resorces are not fetched if not yet expired. + */ + @Test + public void testResourcesNotExprired() throws AcmeException { + ZonedDateTime expiryDate = ZonedDateTime.now().plus(60, ChronoUnit.DAYS); + + final Connection connection = mock(Connection.class); + final Session session = mock(Session.class); + + when(session.getDirectoryExpires()).thenReturn(expiryDate); + + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(); + JSON map = provider.directory(session, SERVER_URI); + + assertThat(map, is(nullValue())); + + verify(session).getDirectoryExpires(); + verifyNoMoreInteractions(session); + + verifyNoMoreInteractions(connection); + } + + /** + * Verify that resorces are fetched if expired. + */ + @Test + public void testResourcesExprired() throws AcmeException { + ZonedDateTime expiryDate = ZonedDateTime.now().plus(60, ChronoUnit.DAYS); + ZonedDateTime pastExpiryDate = ZonedDateTime.now().minus(10, ChronoUnit.MINUTES); + + final Connection connection = mock(Connection.class); + final Session session = mock(Session.class); + + when(connection.readJsonResponse()).thenReturn(getJSON("directory")); + when(connection.getExpiration()).thenReturn(Optional.of(expiryDate)); + when(connection.getLastModified()).thenReturn(Optional.empty()); + when(session.getDirectoryExpires()).thenReturn(pastExpiryDate); + + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(connection); + JSON map = provider.directory(session, SERVER_URI); + + assertThat(map.toString(), sameJSONAs(TestUtils.getJSON("directory").toString())); + + verify(session).setDirectoryExpires(eq(expiryDate)); + verify(session).setDirectoryLastModified(eq(null)); + verify(session).getDirectoryExpires(); + verify(session).getDirectoryLastModified(); + verifyNoMoreInteractions(session); + + verify(connection).sendRequest(RESOLVED_URL, session, null); + verify(connection).getNonce(); + verify(connection).getLastModified(); + verify(connection).getExpiration(); + verify(connection).readJsonResponse(); + verify(connection).close(); + verifyNoMoreInteractions(connection); + } + + /** + * Verify that if-modified-since is used. + */ + @Test + public void testResourcesIfModifiedSince() throws AcmeException { + ZonedDateTime modifiedSinceDate = ZonedDateTime.now().minus(60, ChronoUnit.DAYS); + + final Connection connection = mock(Connection.class); + final Session session = mock(Session.class); + + when(connection.sendRequest(eq(RESOLVED_URL), eq(session), eq(modifiedSinceDate))) + .thenReturn(HttpURLConnection.HTTP_NOT_MODIFIED); + when(connection.getLastModified()).thenReturn(Optional.of(modifiedSinceDate)); + when(session.getDirectoryLastModified()).thenReturn(modifiedSinceDate); + + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(connection); + JSON map = provider.directory(session, SERVER_URI); + + assertThat(map, is(nullValue())); + + verify(session).getDirectoryExpires(); + verify(session).getDirectoryLastModified(); + verifyNoMoreInteractions(session); + + verify(connection).sendRequest(RESOLVED_URL, session, modifiedSinceDate); + verify(connection).close(); + verifyNoMoreInteractions(connection); + } + + /** + * Verify that HTTP errors are handled correctly. + */ + @Test + public void testResourcesHttpError() throws AcmeException, IOException { + final HttpURLConnection conn = mock(HttpURLConnection.class); + final HttpConnector connector = mock(HttpConnector.class); + final Connection connection = new DefaultConnection(connector); + + when(connector.openConnection(any(), any())).thenReturn(conn); + when(conn.getResponseCode()).thenReturn(HttpURLConnection.HTTP_INTERNAL_ERROR); + when(conn.getResponseMessage()).thenReturn("Internal error"); + + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(connection); + Session session = TestUtils.session(provider); + + try { + provider.directory(session, SERVER_URI); + fail("HTTP error was ignored"); + } catch (AcmeException ex) { + // expected + } + } + /** * Test that challenges are generated properly. */ @@ -128,17 +258,7 @@ public class AbstractAcmeProviderTest { public void testCreateChallenge() { Login login = mock(Login.class); - AbstractAcmeProvider provider = new AbstractAcmeProvider() { - @Override - public boolean accepts(URI serverUri) { - throw new UnsupportedOperationException(); - } - - @Override - public URL resolve(URI serverUri) { - throw new UnsupportedOperationException(); - } - }; + AbstractAcmeProvider provider = new TestAbstractAcmeProvider(); Challenge c1 = provider.createChallenge(login, getJSON("httpChallenge")); assertThat(c1, not(nullValue())); @@ -190,4 +310,35 @@ public class AbstractAcmeProviderTest { } } + @ParametersAreNonnullByDefault + private static class TestAbstractAcmeProvider extends AbstractAcmeProvider { + private final Connection connection; + + public TestAbstractAcmeProvider() { + this.connection = null; + } + + public TestAbstractAcmeProvider(Connection connection) { + this.connection = connection; + } + + @Override + public boolean accepts(URI serverUri) { + assertThat(serverUri, is(SERVER_URI)); + return true; + } + + @Override + public URL resolve(URI serverUri) { + assertThat(serverUri, is(SERVER_URI)); + return RESOLVED_URL; + } + + @Override + public Connection connect(URI serverUri) { + assertThat(serverUri, is(SERVER_URI)); + return connection != null ? connection : super.connect(serverUri); + } + } + } diff --git a/src/doc/docs/migration.md b/src/doc/docs/migration.md index 6a4db34a..70e00a9d 100644 --- a/src/doc/docs/migration.md +++ b/src/doc/docs/migration.md @@ -2,6 +2,11 @@ This document will help you migrate your code to the latest _acme4j_ version. +## Migration to Version 2.10 + +- When fetching the directory, acme4j now evaluates HTTP caching headers instead of just caching the directory for 1 hour. However, Let's Encrypt explicitly forbids caching, which means that a fresh copy of the directory is now fetched from the server every time it is needed. I don't like it, but it is the RFC conformous behavior. It needs to be [fixed on Let's Encrypt side](https://github.com/letsencrypt/boulder/issues/4814). +- `AcmeProvider.directory(Session, URI)` is now responsible for maintaining the cache. Implementations can use `Session.setDirectoryExpires()`, `Session.setDirectoryLastModified()`, and the respective getters, for keeping track of the local directory state. `AcmeProvider.directory(Session, URI)` may now return `null`, to indicate that the remote directory was unchanged and the local copy is still valid. It's not permitted to return `null` if `Session.hasDirectory()` returns `false`, though! If your `AcmeProvider` is derived from `AbstractAcmeProvider`, and you haven't overridden the `directory()` method, no migration is necessary. + ## Migration to Version 2.9 - In the ACME STAR draft 09, the term "recurring" has been changed to "auto-renewal". To reflect this change, all STAR related methods in the acme4j API have been renamed as well. If you are using the STAR extension, you are going to get a number of compile errors, but you will always find a corresponding new method. No functionality has been removed. I decided to do a hard API change because acme4j's STAR support is still experimental.