Avoid URI to URL conversion

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);
-
+                alternates = new ArrayList<>(conn.getLinks("alternate"));
-                Collection<URI> alternateList = conn.getLinks("alternate");
-                if (alternateList != null) {
-                    alternates = alternateList.stream()
-                             .map(AcmeUtils::toURL)
-                             .collect(toCollection(ArrayList::new));
-                }
-
                 certChain = new ArrayList<>(conn.readCertificates());
             }
         }

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.
+     * Gets one or more relation links from the header. The result is expected to be an URL.
-     * <p>
-     * 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.
      * <p>
      * 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<URI> getLinks(String relation);
+    Collection<URL> getLinks(String relation);
 
     /**
      * Closes the {@link Connection}, releasing all resources.

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) {
+    public Collection<URL> getLinks(String relation) {
-        Collection<URI> links = getLinks(relation);
+        return collectLinks(relation).stream()
-        if (links == null) {
+                .map(this::resolveRelative)
-            return null;
+                .collect(toList());
-        }
-
-        if (links.size() > 1) {
-            LOG.debug("Link: {} - using the first of {}", relation, links.size());
-        }
-
-        return toURL(links.iterator().next());
-    }
-
-    @Override
-    public Collection<URI> getLinks(String relation) {
-        assertConnectionIsOpen();
-
-        List<URI> result = new ArrayList<>();
-
-        List<String> 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;
     }
 
     @Override
@@ -420,14 +392,22 @@ public class DefaultConnection implements Connection {
         }
 
         if ("userActionRequired".equals(error)) {
-            Collection<URI> links = getLinks("terms-of-service");
+            URI tos = collectLinks("terms-of-service").stream()
-            URI tos = links != null ? links.stream().findFirst().orElse(null) : null;
+                    .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<Instant> retryAfter = getRetryAfterHeader();
-            Collection<URI> rateLimits = getLinks("urn:ietf:params:acme:documentation");
+            Collection<URL> 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<String> collectLinks(String relation) {
+        assertConnectionIsOpen();
+
+        List<String> result = new ArrayList<>();
+
+        List<String> 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);
+            return new URL(conn.getURL(), link);
-        } catch (URISyntaxException ex) {
+        } catch (MalformedURLException ex) {
             throw new AcmeProtocolException("Cannot resolve relative link: " + link, ex);
         }
     }

acme4j-client/src/main/java/org/shredzone/acme4j/connector/ResourceIterator.java

@@ -146,7 +146,7 @@ public class ResourceIterator<T extends AcmeResource> implements Iterator<T> {
             JSON json = conn.readJsonResponse();
             fillUrlList(json);
 
-            nextUrl = conn.getLink("next");
+            nextUrl = conn.getLinks("next").stream().findFirst().orElse(null);
         }
     }
 

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<URI> documents;
+    private final Collection<URL> 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<URI> documents) {
+    public AcmeRateLimitedException(Problem problem, Instant retryAfter, Collection<URL> 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.
+     * Collection of URLs pointing to documents about the rate limit that was hit.
-     * {@code null} if the server did not provide such URIs.
+     * {@code null} if the server did not provide such URLs.
      */
-    public Collection<URI> getDocuments() {
+    public Collection<URL> getDocuments() {
         return documents;
     }
 

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.
      *

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) {
+            public Collection<URL> getLinks(String relation) {
-                return null;
+                return Collections.emptyList();
-            }
-
-            @Override
-            public Collection<URI> getLinks(String relation) {
-                switch(relation) {
-                    case "terms-of-service": return Arrays.asList(agreementUri);
-                    default: return null;
-                }
             }
         };
 
@@ -168,14 +161,9 @@ public class AccountTest {
             }
 
             @Override
-            public URL getLink(String relation) {
+            public Collection<URL> getLinks(String relation) {
-                return null;
-            }
-
-            @Override
-            public Collection<URI> getLinks(String relation) {
                 switch(relation) {
-                    case "terms-of-service": return Arrays.asList(agreementUri);
+                    case "terms-of-service": return Arrays.asList(agreementUrl);
                     default: return null;
                 }
             }

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<URI> getLinks(String relation) {
+            public Collection<URL> getLinks(String relation) {
                 assertThat(relation, is("alternate"));
                 return Arrays.asList(
-                        URI.create("https://example.com/acme/alt-cert/1"),
+                        url("https://example.com/acme/alt-cert/1"),
-                        URI.create("https://example.com/acme/alt-cert/2"));
+                        url("https://example.com/acme/alt-cert/2"));
             }
         };
 
@@ -159,9 +160,9 @@ public class CertificateTest {
             }
 
             @Override
-            public Collection<URI> getLinks(String relation) {
+            public Collection<URL> getLinks(String relation) {
                 assertThat(relation, is("alternate"));
-                return null;
+                return Collections.emptyList();
             }
         };
 
@@ -212,9 +213,9 @@ public class CertificateTest {
             }
 
             @Override
-            public Collection<URI> getLinks(String relation) {
+            public Collection<URL> getLinks(String relation) {
                 assertThat(relation, is("alternate"));
-                return null;
+                return Collections.emptyList();
             }
         };
 

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.getLinks("next"), containsInAnyOrder(new URL("https://example.com/acme/new-authz")));
-            assertThat(conn.getLink("recover"), is(new URL("https://example.org/recover-acct")));
+            assertThat(conn.getLinks("recover"), containsInAnyOrder(new URL("https://example.org/recover-acct")));
-            assertThat(conn.getLink("terms-of-service"), is(new URL("https://example.com/acme/terms")));
+            assertThat(conn.getLinks("terms-of-service"), containsInAnyOrder(new URL("https://example.com/acme/terms")));
-            assertThat(conn.getLink("secret-stuff"), is(nullValue()));
+            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<String, List<String>> 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"),
+                        url("https://example.com/acme/terms1"),
-                        URI.create("https://example.com/acme/terms2"),
+                        url("https://example.com/acme/terms2"),
-                        URI.create("https://example.com/acme/terms3")
+                        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");
         }

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) {
+    public Collection<URL> getLinks(String relation) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public Collection<URI> getLinks(String relation) {
         throw new UnsupportedOperationException();
     }
 

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

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<URI> documents = Arrays.asList(
+        Collection<URL> documents = Arrays.asList(
-                        URI.create("http://example.com/doc1.html"),
+                        url("http://example.com/doc1.html"),
-                        URI.create("http://example.com/doc2.html"));
+                        url("http://example.com/doc2.html"));
 
         Problem problem = createProblem(type, detail, null);
 

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)}.
      */