From e9a330b3a2319f97e4fc5d2793f265c6ca2e17b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20K=C3=B6rber?= Date: Sat, 4 Nov 2017 11:40:20 +0100 Subject: [PATCH] Avoid URI to URL conversion --- .../org/shredzone/acme4j/Certificate.java | 11 +-- .../acme4j/connector/Connection.java | 19 +--- .../acme4j/connector/DefaultConnection.java | 94 ++++++++++--------- .../acme4j/connector/ResourceIterator.java | 2 +- .../exception/AcmeRateLimitedException.java | 14 +-- .../shredzone/acme4j/toolbox/AcmeUtils.java | 20 ---- .../org/shredzone/acme4j/AccountTest.java | 24 ++--- .../org/shredzone/acme4j/CertificateTest.java | 15 +-- .../connector/DefaultConnectionTest.java | 20 ++-- .../acme4j/connector/DummyConnection.java | 8 +- .../connector/ResourceIteratorTest.java | 9 +- .../AcmeRateLimitedExceptionTest.java | 9 +- .../acme4j/toolbox/AcmeUtilsTest.java | 15 --- 13 files changed, 99 insertions(+), 161 deletions(-) diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java b/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java index ce2fdf3d..72288268 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/Certificate.java @@ -14,7 +14,6 @@ package org.shredzone.acme4j; import static java.util.Collections.unmodifiableList; -import static java.util.stream.Collectors.toCollection; import java.io.IOException; import java.io.Writer; @@ -25,7 +24,6 @@ import java.security.KeyPair; import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.function.BiFunction; @@ -85,14 +83,7 @@ public class Certificate extends AcmeResource { try (Connection conn = getSession().provider().connect()) { conn.sendRequest(getLocation(), getSession()); conn.accept(HttpURLConnection.HTTP_OK); - - Collection alternateList = conn.getLinks("alternate"); - if (alternateList != null) { - alternates = alternateList.stream() - .map(AcmeUtils::toURL) - .collect(toCollection(ArrayList::new)); - } - + alternates = new ArrayList<>(conn.getLinks("alternate")); certChain = new ArrayList<>(conn.readCertificates()); } } 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 41db8a70..5876a044 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 @@ -13,7 +13,6 @@ */ package org.shredzone.acme4j.connector; -import java.net.URI; import java.net.URL; import java.security.cert.X509Certificate; import java.util.Collection; @@ -130,27 +129,15 @@ public interface Connection extends AutoCloseable { URL getLocation(); /** - * Gets a relation link from the header. The result is expected to be an URL. - *

- * Relative links are resolved against the last request's URL. If there is more than - * one relation, the first one is returned. - * - * @param relation - * Link relation - * @return Link, or {@code null} if there was no such relation link - */ - URL getLink(String relation); - - /** - * Gets one or more relation links from the header. + * Gets one or more relation links from the header. The result is expected to be an URL. *

* Relative links are resolved against the last request's URL. * * @param relation * Link relation - * @return Collection of links, or {@code null} if there was no such relation link + * @return Collection of links. Empty if there was no such relation. */ - Collection getLinks(String relation); + Collection getLinks(String relation); /** * Closes the {@link Connection}, releasing all resources. 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 9cc19a04..0ce99c5c 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 @@ -14,12 +14,13 @@ package org.shredzone.acme4j.connector; import static java.util.stream.Collectors.toList; -import static org.shredzone.acme4j.toolbox.AcmeUtils.*; +import static org.shredzone.acme4j.toolbox.AcmeUtils.keyAlgorithm; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.HttpURLConnection; +import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; @@ -331,43 +332,14 @@ public class DefaultConnection implements Connection { } LOG.debug("Location: {}", location); - return toURL(resolveRelative(location)); + return resolveRelative(location); } @Override - public URL getLink(String relation) { - Collection links = getLinks(relation); - if (links == null) { - return null; - } - - if (links.size() > 1) { - LOG.debug("Link: {} - using the first of {}", relation, links.size()); - } - - return toURL(links.iterator().next()); - } - - @Override - public Collection getLinks(String relation) { - assertConnectionIsOpen(); - - List result = new ArrayList<>(); - - List links = conn.getHeaderFields().get(LINK_HEADER); - if (links != null) { - Pattern p = Pattern.compile("<(.*?)>\\s*;\\s*rel=\"?"+ Pattern.quote(relation) + "\"?"); - for (String link : links) { - Matcher m = p.matcher(link); - if (m.matches()) { - String location = m.group(1); - LOG.debug("Link: {} -> {}", relation, location); - result.add(resolveRelative(location)); - } - } - } - - return !result.isEmpty() ? result : null; + public Collection getLinks(String relation) { + return collectLinks(relation).stream() + .map(this::resolveRelative) + .collect(toList()); } @Override @@ -420,14 +392,22 @@ public class DefaultConnection implements Connection { } if ("userActionRequired".equals(error)) { - Collection links = getLinks("terms-of-service"); - URI tos = links != null ? links.stream().findFirst().orElse(null) : null; + URI tos = collectLinks("terms-of-service").stream() + .findFirst() + .map(it -> { + try { + return conn.getURL().toURI().resolve(it); + } catch (URISyntaxException ex) { + throw new AcmeProtocolException("Invalid TOS URI", ex); + } + }) + .orElse(null); return new AcmeUserActionRequiredException(problem, tos); } if ("rateLimited".equals(error)) { Optional retryAfter = getRetryAfterHeader(); - Collection rateLimits = getLinks("urn:ietf:params:acme:documentation"); + Collection rateLimits = getLinks("urn:ietf:params:acme:documentation"); return new AcmeRateLimitedException(problem, retryAfter.orElse(null), rateLimits); } @@ -467,24 +447,52 @@ public class DefaultConnection implements Connection { ); } + /** + * Collects links of the given relation. + * + * @param relation + * Link relation + * @return Collection of links, unconverted + */ + private Collection collectLinks(String relation) { + assertConnectionIsOpen(); + + List result = new ArrayList<>(); + + List links = conn.getHeaderFields().get(LINK_HEADER); + if (links != null) { + Pattern p = Pattern.compile("<(.*?)>\\s*;\\s*rel=\"?"+ Pattern.quote(relation) + "\"?"); + for (String link : links) { + Matcher m = p.matcher(link); + if (m.matches()) { + String location = m.group(1); + LOG.debug("Link: {} -> {}", relation, location); + result.add(location); + } + } + } + + return result; + } + /** * Resolves a relative link against the connection's last URL. * * @param link - * Link to resolve. Absolute links are just converted to an URI. May be + * Link to resolve. Absolute links are just converted to an URL. May be * {@code null}. - * @return Absolute URI of the given link, or {@code null} if the link was + * @return Absolute URL of the given link, or {@code null} if the link was * {@code null}. */ - private URI resolveRelative(String link) { + private URL resolveRelative(String link) { if (link == null) { return null; } assertConnectionIsOpen(); try { - return conn.getURL().toURI().resolve(link); - } catch (URISyntaxException ex) { + return new URL(conn.getURL(), link); + } catch (MalformedURLException ex) { throw new AcmeProtocolException("Cannot resolve relative link: " + link, ex); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java index df4ec7aa..2be55688 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java @@ -146,7 +146,7 @@ public class ResourceIterator implements Iterator { JSON json = conn.readJsonResponse(); fillUrlList(json); - nextUrl = conn.getLink("next"); + nextUrl = conn.getLinks("next").stream().findFirst().orElse(null); } } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRateLimitedException.java b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRateLimitedException.java index ca1d13ad..93c88f65 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRateLimitedException.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/exception/AcmeRateLimitedException.java @@ -13,7 +13,7 @@ */ package org.shredzone.acme4j.exception; -import java.net.URI; +import java.net.URL; import java.time.Instant; import java.util.Collection; import java.util.Collections; @@ -27,7 +27,7 @@ public class AcmeRateLimitedException extends AcmeServerException { private static final long serialVersionUID = 4150484059796413069L; private final Instant retryAfter; - private final Collection documents; + private final Collection documents; /** * Creates a new {@link AcmeRateLimitedException}. @@ -38,9 +38,9 @@ public class AcmeRateLimitedException extends AcmeServerException { * The moment the request is expected to succeed again, may be {@code null} * if not known * @param documents - * URIs pointing to documents about the rate limit that was hit + * URLs pointing to documents about the rate limit that was hit */ - public AcmeRateLimitedException(Problem problem, Instant retryAfter, Collection documents) { + public AcmeRateLimitedException(Problem problem, Instant retryAfter, Collection documents) { super(problem); this.retryAfter = retryAfter; this.documents = @@ -56,10 +56,10 @@ public class AcmeRateLimitedException extends AcmeServerException { } /** - * Collection of URIs pointing to documents about the rate limit that was hit. - * {@code null} if the server did not provide such URIs. + * Collection of URLs pointing to documents about the rate limit that was hit. + * {@code null} if the server did not provide such URLs. */ - public Collection getDocuments() { + public Collection getDocuments() { return documents; } diff --git a/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/AcmeUtils.java b/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/AcmeUtils.java index 2fd75083..29bc50fe 100644 --- a/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/AcmeUtils.java +++ b/acme4j-client/src/main/java/org/shredzone/acme4j/toolbox/AcmeUtils.java @@ -17,9 +17,6 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.io.Writer; import java.net.IDN; -import java.net.MalformedURLException; -import java.net.URI; -import java.net.URL; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Instant; @@ -312,23 +309,6 @@ public final class AcmeUtils { out.append("\n-----END ").append(label.toString()).append("-----\n"); } - /** - * Converts {@link URI} to {@link URL}. - * - * @param uri - * {@link URI} to convert - * @return {@link URL} - * @throws AcmeProtocolException - * if the URI could not be converted to URL - */ - public static URL toURL(URI uri) { - try { - return uri != null ? uri.toURL() : null; - } catch (MalformedURLException ex) { - throw new AcmeProtocolException("Invalid URL: " + uri, ex); - } - } - /** * Extracts the content type of a Content-Type header. * diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java index 34f07d3e..c4c05da6 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/AccountTest.java @@ -28,6 +28,7 @@ import java.security.KeyPair; import java.time.Instant; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.concurrent.atomic.AtomicBoolean; @@ -55,7 +56,7 @@ public class AccountTest { private URL resourceUrl = url("http://example.com/acme/resource"); private URL locationUrl = url("http://example.com/acme/account"); - private URI agreementUri = URI.create("http://example.com/agreement.pdf"); + private URL agreementUrl = url("http://example.com/agreement.pdf"); /** * Test that a account can be updated. @@ -105,16 +106,8 @@ public class AccountTest { } @Override - public URL getLink(String relation) { - return null; - } - - @Override - public Collection getLinks(String relation) { - switch(relation) { - case "terms-of-service": return Arrays.asList(agreementUri); - default: return null; - } + public Collection getLinks(String relation) { + return Collections.emptyList(); } }; @@ -168,14 +161,9 @@ public class AccountTest { } @Override - public URL getLink(String relation) { - return null; - } - - @Override - public Collection getLinks(String relation) { + public Collection getLinks(String relation) { switch(relation) { - case "terms-of-service": return Arrays.asList(agreementUri); + case "terms-of-service": return Arrays.asList(agreementUrl); default: return null; } } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java index 99d9fdd7..eaed7cef 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/CertificateTest.java @@ -29,6 +29,7 @@ import java.security.KeyPair; import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import org.junit.Test; @@ -72,11 +73,11 @@ public class CertificateTest { } @Override - public Collection getLinks(String relation) { + public Collection getLinks(String relation) { assertThat(relation, is("alternate")); return Arrays.asList( - URI.create("https://example.com/acme/alt-cert/1"), - URI.create("https://example.com/acme/alt-cert/2")); + url("https://example.com/acme/alt-cert/1"), + url("https://example.com/acme/alt-cert/2")); } }; @@ -159,9 +160,9 @@ public class CertificateTest { } @Override - public Collection getLinks(String relation) { + public Collection getLinks(String relation) { assertThat(relation, is("alternate")); - return null; + return Collections.emptyList(); } }; @@ -212,9 +213,9 @@ public class CertificateTest { } @Override - public Collection getLinks(String relation) { + public Collection getLinks(String relation) { assertThat(relation, is("alternate")); - return null; + return Collections.emptyList(); } }; 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 78a2120b..70a454e9 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 @@ -246,10 +246,10 @@ public class DefaultConnectionTest { try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { conn.conn = mockUrlConnection; - assertThat(conn.getLink("next"), is(new URL("https://example.com/acme/new-authz"))); - assertThat(conn.getLink("recover"), is(new URL("https://example.org/recover-acct"))); - assertThat(conn.getLink("terms-of-service"), is(new URL("https://example.com/acme/terms"))); - assertThat(conn.getLink("secret-stuff"), is(nullValue())); + assertThat(conn.getLinks("next"), containsInAnyOrder(new URL("https://example.com/acme/new-authz"))); + assertThat(conn.getLinks("recover"), containsInAnyOrder(new URL("https://example.org/recover-acct"))); + assertThat(conn.getLinks("terms-of-service"), containsInAnyOrder(new URL("https://example.com/acme/terms"))); + assertThat(conn.getLinks("secret-stuff"), is(empty())); } } @@ -258,7 +258,7 @@ public class DefaultConnectionTest { */ @Test public void testGetMultiLink() { - URL baseUrl = TestUtils.url("https://example.com/acme/request/1234"); + URL baseUrl = url("https://example.com/acme/request/1234"); Map> headers = new HashMap<>(); headers.put("Link", Arrays.asList( @@ -273,9 +273,9 @@ public class DefaultConnectionTest { try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { conn.conn = mockUrlConnection; assertThat(conn.getLinks("terms-of-service"), containsInAnyOrder( - URI.create("https://example.com/acme/terms1"), - URI.create("https://example.com/acme/terms2"), - URI.create("https://example.com/acme/terms3") + url("https://example.com/acme/terms1"), + url("https://example.com/acme/terms2"), + url("https://example.com/acme/terms3") )); } } @@ -290,7 +290,7 @@ public class DefaultConnectionTest { try (DefaultConnection conn = new DefaultConnection(mockHttpConnection)) { conn.conn = mockUrlConnection; - assertThat(conn.getLinks("something"), is(nullValue())); + assertThat(conn.getLinks("something"), is(empty())); } } @@ -511,7 +511,7 @@ public class DefaultConnectionTest { assertThat(ex.getRetryAfter(), is(retryAfter)); assertThat(ex.getDocuments(), is(notNullValue())); assertThat(ex.getDocuments().size(), is(1)); - assertThat(ex.getDocuments().iterator().next(), is(URI.create("https://example.com/rates.pdf"))); + assertThat(ex.getDocuments().iterator().next(), is(url("https://example.com/rates.pdf"))); } catch (AcmeException ex) { fail("Expected an AcmeRateLimitedException"); } 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 b6f935c2..4bef9178 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 @@ -13,7 +13,6 @@ */ package org.shredzone.acme4j.connector; -import java.net.URI; import java.net.URL; import java.security.cert.X509Certificate; import java.util.Collection; @@ -82,12 +81,7 @@ public class DummyConnection implements Connection { } @Override - public URL getLink(String relation) { - throw new UnsupportedOperationException(); - } - - @Override - public Collection getLinks(String relation) { + public Collection getLinks(String relation) { throw new UnsupportedOperationException(); } diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/ResourceIteratorTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/ResourceIteratorTest.java index 82205fe1..259034c2 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/connector/ResourceIteratorTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/connector/ResourceIteratorTest.java @@ -21,6 +21,9 @@ import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.NoSuchElementException; @@ -155,11 +158,11 @@ public class ResourceIteratorTest { } @Override - public URL getLink(String relation) { + public Collection getLinks(String relation) { if ("next".equals(relation) && (ix + 1 < pageURLs.size())) { - return pageURLs.get(ix + 1); + return Arrays.asList(pageURLs.get(ix + 1)); } - return null; + return Collections.emptyList(); } }; diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/exception/AcmeRateLimitedExceptionTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/exception/AcmeRateLimitedExceptionTest.java index fd11440f..379b1fbb 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/exception/AcmeRateLimitedExceptionTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/exception/AcmeRateLimitedExceptionTest.java @@ -15,9 +15,10 @@ package org.shredzone.acme4j.exception; import static org.hamcrest.Matchers.*; import static org.junit.Assert.assertThat; -import static org.shredzone.acme4j.toolbox.TestUtils.createProblem; +import static org.shredzone.acme4j.toolbox.TestUtils.*; import java.net.URI; +import java.net.URL; import java.time.Duration; import java.time.Instant; import java.util.Arrays; @@ -39,9 +40,9 @@ public class AcmeRateLimitedExceptionTest { URI type = URI.create("urn:ietf:params:acme:error:rateLimited"); String detail = "Too many requests per minute"; Instant retryAfter = Instant.now().plus(Duration.ofMinutes(1)); - Collection documents = Arrays.asList( - URI.create("http://example.com/doc1.html"), - URI.create("http://example.com/doc2.html")); + Collection documents = Arrays.asList( + url("http://example.com/doc1.html"), + url("http://example.com/doc2.html")); Problem problem = createProblem(type, detail, null); diff --git a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/AcmeUtilsTest.java b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/AcmeUtilsTest.java index 50cb6146..ba4bdb3f 100644 --- a/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/AcmeUtilsTest.java +++ b/acme4j-client/src/test/java/org/shredzone/acme4j/toolbox/AcmeUtilsTest.java @@ -24,9 +24,6 @@ import java.io.OutputStreamWriter; import java.io.Writer; import java.lang.reflect.Constructor; import java.lang.reflect.Modifier; -import java.net.MalformedURLException; -import java.net.URI; -import java.net.URL; import java.security.KeyPair; import java.security.Security; import java.security.cert.CertificateEncodingException; @@ -288,18 +285,6 @@ public class AcmeUtilsTest { assertThat(pemFile.toByteArray(), is(originalFile.toByteArray())); } - /** - * Test {@link AcmeUtils#toURL(URI)}. - */ - @Test - public void testToURL() throws MalformedURLException { - URI testUri = URI.create("https://example.com/foo/123"); - URL testUrl = testUri.toURL(); - - assertThat(AcmeUtils.toURL(testUri), is(testUrl)); - assertThat(AcmeUtils.toURL(null), is(nullValue())); - } - /** * Test {@link AcmeUtils#getContentType(String)}. */