getLocation() throws if header is missing

While reviewing the code, it turned out that the invoker of
getLocation() always expected to find a location header. To keep the API
simple, getLocation() now always returns the non-null Location header.
If the header is not present, an AcmeProtocolException is now thrown
instead.
pull/140/head
Richard Körber 2023-05-06 17:15:36 +02:00
parent 23906ff39c
commit aa5e78c525
No known key found for this signature in database
GPG Key ID: AAB9FD19C78AA3E0
10 changed files with 36 additions and 45 deletions

View File

@ -196,9 +196,7 @@ public class Account extends AcmeJsonResource {
conn.sendSignedRequest(newAuthzUrl, claims, getLogin());
var authLocation = conn.getLocation()
.orElseThrow(() -> new AcmeProtocolException("Server did not provide an authorization location"));
var auth = getLogin().bindAuthorization(authLocation);
var auth = getLogin().bindAuthorization(conn.getLocation());
auth.setJSON(conn.readJsonResponse());
return auth;
}

View File

@ -26,7 +26,6 @@ import javax.crypto.spec.SecretKeySpec;
import edu.umd.cs.findbugs.annotations.Nullable;
import org.shredzone.acme4j.connector.Resource;
import org.shredzone.acme4j.exception.AcmeException;
import org.shredzone.acme4j.exception.AcmeProtocolException;
import org.shredzone.acme4j.toolbox.AcmeUtils;
import org.shredzone.acme4j.toolbox.JSONBuilder;
import org.shredzone.acme4j.toolbox.JoseUtils;
@ -261,10 +260,7 @@ public class AccountBuilder {
conn.sendSignedRequest(resourceUrl, claims, session, keyPair);
var location = conn.getLocation()
.orElseThrow(() -> new AcmeProtocolException("Server did not provide an account location"));
var login = new Login(location, keyPair, session);
var login = new Login(conn.getLocation(), keyPair, session);
login.getAccount().setJSON(conn.readJsonResponse());
return login;
}

View File

@ -314,10 +314,7 @@ public class OrderBuilder {
conn.sendSignedRequest(session.resourceUrl(Resource.NEW_ORDER), claims, login);
var orderLocation = conn.getLocation()
.orElseThrow(() -> new AcmeProtocolException("Server did not provide an order location"));
var order = new Order(login, orderLocation);
var order = new Order(login, conn.getLocation());
order.setJSON(conn.readJsonResponse());
return order;
}

View File

@ -169,9 +169,11 @@ public interface Connection extends AutoCloseable {
* <p>
* Relative links are resolved against the last request's URL.
*
* @return Location {@link URL}, or empty if no Location header was set
* @return Location {@link URL}
* @throws org.shredzone.acme4j.exception.AcmeProtocolException if the location
* header is missing
*/
Optional<URL> getLocation();
URL getLocation();
/**
* Returns the content of the last-modified header, if present.

View File

@ -258,14 +258,15 @@ public class DefaultConnection implements Connection {
}
@Override
public Optional<URL> getLocation() {
public URL getLocation() {
return getResponse().headers()
.firstValue(LOCATION_HEADER)
.map(l -> {
LOG.debug("Location: {}", l);
return l;
})
.map(this::resolveRelative);
.map(this::resolveRelative)
.orElseThrow(() -> new AcmeProtocolException("location header is missing"));
}
@Override

View File

@ -21,7 +21,6 @@ import static org.shredzone.acme4j.toolbox.TestUtils.url;
import java.net.HttpURLConnection;
import java.net.URL;
import java.security.KeyPair;
import java.util.Optional;
import org.jose4j.jwx.CompactSerializer;
import org.junit.jupiter.api.Test;
@ -72,8 +71,8 @@ public class AccountBuilderTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
@Override
@ -134,8 +133,8 @@ public class AccountBuilderTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
@Override
@ -176,8 +175,8 @@ public class AccountBuilderTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
@Override

View File

@ -27,7 +27,6 @@ import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URL;
import java.util.Collection;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import org.jose4j.jws.JsonWebSignature;
@ -88,8 +87,8 @@ public class AccountTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
@Override
@ -145,8 +144,8 @@ public class AccountTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
@Override
@ -199,8 +198,8 @@ public class AccountTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
};
@ -327,8 +326,8 @@ public class AccountTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
};
@ -424,8 +423,8 @@ public class AccountTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
};

View File

@ -25,7 +25,6 @@ import java.net.InetAddress;
import java.net.URL;
import java.time.Duration;
import java.util.Arrays;
import java.util.Optional;
import org.assertj.core.api.AutoCloseableSoftAssertions;
import org.junit.jupiter.api.Test;
@ -67,8 +66,8 @@ public class OrderBuilderTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
};
@ -150,8 +149,8 @@ public class OrderBuilderTest {
}
@Override
public Optional<URL> getLocation() {
return Optional.of(locationUrl);
public URL getLocation() {
return locationUrl;
}
};

View File

@ -18,6 +18,7 @@ import static java.time.temporal.ChronoUnit.SECONDS;
import static com.github.tomakehurst.wiremock.client.WireMock.*;
import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.shredzone.acme4j.toolbox.TestUtils.getResourceAsByteArray;
import static org.shredzone.acme4j.toolbox.TestUtils.url;
@ -215,8 +216,7 @@ public class DefaultConnectionTest {
try (var conn = session.connect()) {
conn.sendRequest(requestUrl, session, null);
var location = conn.getLocation();
assertThat(location.orElseThrow())
.isEqualTo(new URL("https://example.com/otherlocation"));
assertThat(location).isEqualTo(new URL("https://example.com/otherlocation"));
}
}
@ -232,8 +232,7 @@ public class DefaultConnectionTest {
try (var conn = session.connect()) {
conn.sendRequest(requestUrl, session, null);
var location = conn.getLocation();
assertThat(location.orElseThrow())
.isEqualTo(new URL(baseUrl + "/otherlocation"));
assertThat(location).isEqualTo(new URL(baseUrl + "/otherlocation"));
}
}
@ -300,7 +299,8 @@ public class DefaultConnectionTest {
try (var conn = session.connect()) {
conn.sendRequest(requestUrl, session, null);
assertThat(conn.getLocation()).isEmpty();
assertThatExceptionOfType(AcmeProtocolException.class)
.isThrownBy(conn::getLocation);
}
verify(getRequestedFor(urlEqualTo(REQUEST_PATH)));

View File

@ -85,7 +85,7 @@ public class DummyConnection implements Connection {
}
@Override
public Optional<URL> getLocation() {
public URL getLocation() {
throw new UnsupportedOperationException();
}