Skip to content

Commit

Permalink
Níma WebClient redirect support (helidon-io#6929)
Browse files Browse the repository at this point in the history
Client redirect support - only HTTP 1.1 for now

Signed-off-by: David Kral <david.k.kral@oracle.com>
  • Loading branch information
Verdent authored and dalexandrov committed Jun 7, 2023
1 parent 9f776e8 commit d5de0d2
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 21 deletions.
5 changes: 5 additions & 0 deletions common/http/src/main/java/io/helidon/common/http/Http.java
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,11 @@ public static class Status {
* <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.8">HTTP/1.1 documentation</a>.
*/
public static final Status TEMPORARY_REDIRECT_307 = new Status(307, "Temporary Redirect", true);
/**
* 308 Permanent Redirect, see
* <a href="https://www.rfc-editor.org/rfc/rfc7538">HTTP Status Code 308 documentation</a>.
*/
public static final Status PERMANENT_REDIRECT_308 = new Status(308, "Permanent Redirect", true);
/**
* 400 Bad Request, see
* <a href="http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.1">HTTP/1.1 documentation</a>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ static void routing(HttpRouting.Builder builder) {
public void testUpload() throws IOException {
Path file = Files.writeString(Files.createTempFile(null, null), "bar\n");
try (Http1ClientResponse response = client.post("/api")
.followRedirects(false)
.submit(WriteableMultiPart.builder()
.addPart(writeablePart("file[]", "foo.txt", file))
.build())) {
Expand All @@ -81,6 +82,7 @@ public void testStreamUpload() throws IOException {
Path file2 = Files.writeString(Files.createTempFile(null, null), "stream foo\n");
try (Http1ClientResponse response = client.post("/api")
.queryParam("stream", "true")
.followRedirects(false)
.submit(WriteableMultiPart
.builder()
.addPart(writeablePart("file[]", "streamed-foo.txt", file))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ class ClientRequestImpl implements Http2ClientRequest {
private Tls tls;
private int priority;
private boolean priorKnowledge;
private boolean followRedirects;
private int requestPrefetch = 0;
private int maxRedirects;
private ClientConnection explicitConnection;
private Duration flowControlTimeout = Duration.ofMillis(100);
private Duration timeout = Duration.ofSeconds(10);
Expand All @@ -84,6 +86,8 @@ class ClientRequestImpl implements Http2ClientRequest {
this.connectionPrefetch = client.prefetch();
this.tls = tls == null || !tls.enabled() ? null : tls;
this.query = query;
this.followRedirects = client.followRedirects();
this.maxRedirects = client.maxRedirects();
}

@Override
Expand Down Expand Up @@ -238,6 +242,20 @@ public Http2ClientRequest fragment(String fragment) {
return this;
}

@Override
public Http2ClientRequest followRedirects(boolean followRedirects) {
// this.followRedirects = followRedirects;
// return this;
throw new UnsupportedOperationException("Not supported in HTTP2 yet");
}

@Override
public Http2ClientRequest maxRedirects(int maxRedirects) {
// this.maxRedirects = maxRedirects;
// return this;
throw new UnsupportedOperationException("Not supported in HTTP2 yet");
}

UriHelper uriHelper() {
return uri;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class ClientRequestImplTest {
private static final Http.HeaderValue REQ_EXPECT_100_HEADER_NAME = Http.Header.createCached(
Http.Header.create("X-Req-Expect100"), "true");
private static final Http.HeaderName REQ_CONTENT_LENGTH_HEADER_NAME = Http.Header.create("X-Req-ContentLength");
private static final String EXPECTED_GET_AFTER_REDIRECT_STRING = "GET after redirect endpoint reached";
private static final long NO_CONTENT_LENGTH = -1L;

private final String baseURI;
Expand All @@ -70,6 +71,10 @@ class ClientRequestImplTest {
@SetUpRoute
static void routing(HttpRules rules) {
rules.put("/test", ClientRequestImplTest::responseHandler);
rules.put("/redirectKeepMethod", ClientRequestImplTest::redirectKeepMethod);
rules.put("/redirect", ClientRequestImplTest::redirect);
rules.get("/afterRedirect", ClientRequestImplTest::afterRedirectGet);
rules.put("/afterRedirect", ClientRequestImplTest::afterRedirectPut);
rules.put("/chunkresponse", ClientRequestImplTest::chunkResponseHandler);
}

Expand Down Expand Up @@ -283,6 +288,35 @@ void testConnectionQueueSizeLimit() {
assertThat(connectionNow, is(connection));
}

@Test
void testRedirect() {
try (Http1ClientResponse response = injectedHttp1client.put("/redirect")
.followRedirects(false)
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.FOUND_302));
}

try (Http1ClientResponse response = injectedHttp1client.put("/redirect")
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.OK_200));
assertThat(response.as(String.class), is(EXPECTED_GET_AFTER_REDIRECT_STRING));
}
}

@Test
void testRedirectKeepMethod() {
try (Http1ClientResponse response = injectedHttp1client.put("/redirectKeepMethod")
.followRedirects(false)
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.TEMPORARY_REDIRECT_307));
}

try (Http1ClientResponse response = injectedHttp1client.put("/redirectKeepMethod")
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.NO_CONTENT_204));
}
}

private static void validateSuccessfulResponse(Http1Client client) {
String requestEntity = "Sending Something";
Http1ClientRequest request = client.put("/test");
Expand Down Expand Up @@ -315,6 +349,38 @@ private static void validateChunkTransfer(Http1ClientResponse response, boolean
assertThat(responseEntity, is(entity));
}

private static void redirect(ServerRequest req, ServerResponse res) {
res.status(Http.Status.FOUND_302)
.header(Http.Header.LOCATION, "/afterRedirect")
.send();
}

private static void redirectKeepMethod(ServerRequest req, ServerResponse res) {
res.status(Http.Status.TEMPORARY_REDIRECT_307)
.header(Http.Header.LOCATION, "/afterRedirect")
.send();
}

private static void afterRedirectGet(ServerRequest req, ServerResponse res) {
if (req.content().hasEntity()) {
res.status(Http.Status.BAD_REQUEST_400)
.send("GET after redirect endpoint reached with entity");
return;
}
res.send(EXPECTED_GET_AFTER_REDIRECT_STRING);
}

private static void afterRedirectPut(ServerRequest req, ServerResponse res) {
String entity = req.content().as(String.class);
if (!entity.equals("Test entity")) {
res.status(Http.Status.BAD_REQUEST_400)
.send("Entity was not kept the same after the redirect");
return;
}
res.status(Http.Status.NO_CONTENT_204)
.send();
}

private static void responseHandler(ServerRequest req, ServerResponse res) throws IOException {
customHandler(req, res, false);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2022 Oracle and/or its affiliates.
* Copyright (c) 2017, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -42,6 +42,7 @@
*/
class MultiPortTest {
private static final Http1Client CLIENT = WebClient.builder()
.followRedirect(false)
.build();
private Handler commonHandler;
private WebServer server;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,20 @@ public interface ClientConfig {
*/
DnsAddressLookup dnsAddressLookup();

/**
* Whether to follow redirects.
*
* @return follow redirects
*/
@ConfiguredOption("true")
boolean followRedirects();

/**
* Maximum number of redirects allowed.
*
* @return allowed number of redirects
*/
@ConfiguredOption("5")
int maxRedirects();

}
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,22 @@ default B header(Http.HeaderName name, String value) {
*/
B fragment(String fragment);

/**
* Whether to follow redirects.
*
* @param followRedirects follow redirects
* @return updated request
*/
B followRedirects(boolean followRedirects);

/**
* Max number of the followed redirects.
*
* @param maxRedirects max followed redirects
* @return updated request
*/
B maxRedirects(int maxRedirects);

/**
* Request without an entity.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class LoomClient implements WebClient {
private final SocketOptions channelOptions;
private final DnsResolver dnsResolver;
private final DnsAddressLookup dnsAddressLookup;
private final int maxRedirects;
private final boolean followRedirects;

/**
* Construct this instance from a subclass of builder.
Expand All @@ -53,6 +55,8 @@ protected LoomClient(WebClient.Builder<?, ?> builder) {
this.channelOptions = builder.channelOptions() == null ? EMPTY_OPTIONS : builder.channelOptions();
this.dnsResolver = builder.dnsResolver();
this.dnsAddressLookup = builder.dnsAddressLookup();
this.maxRedirects = builder.maxRedirect();
this.followRedirects = builder.followRedirect();
}

/**
Expand Down Expand Up @@ -101,11 +105,30 @@ public DnsResolver dnsResolver() {
}

/**
*
* DNS address lookup instance to be used for this client.
*
* @return DNS address lookup instance type
*/
public DnsAddressLookup dnsAddressLookup() {
return dnsAddressLookup;
}

/**
* Whether to follow redirects.
*
* @return follow redirects
*/
public boolean followRedirects() {
return followRedirects;
}

/**
* Maximum number of redirects allowed.
*
* @return allowed number of redirects
*/
public int maxRedirects() {
return maxRedirects;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public void scheme(String scheme) {
*/
public void host(String host) {
this.host = host;
authority(host, port);
}

/**
Expand All @@ -119,6 +120,7 @@ public void host(String host) {
*/
public void port(int port) {
this.port = port;
authority(host, port);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022 Oracle and/or its affiliates.
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -64,6 +64,8 @@ abstract class Builder<B extends Builder<B, C>, C extends WebClient> implements
private SocketOptions channelOptions;
private DnsResolver dnsResolver;
private DnsAddressLookup dnsAddressLookup;
private boolean followRedirect;
private int maxRedirect;

/**
* Common builder base for all the client builder.
Expand All @@ -89,7 +91,7 @@ public B baseUri(String baseUri) {
*/
public B baseUri(URI baseUri) {
this.baseUri = baseUri;
return (B) this;
return identity();
}

/**
Expand All @@ -102,7 +104,7 @@ public B baseUri(URI baseUri) {
*/
public B tls(Tls tls) {
this.tls = tls;
return (B) this;
return identity();
}

/**
Expand All @@ -115,7 +117,7 @@ public B tls(Tls tls) {
*/
public B tls(Supplier<Tls> tls) {
this.tls = tls.get();
return (B) this;
return identity();
}

/**
Expand All @@ -126,7 +128,7 @@ public B tls(Supplier<Tls> tls) {
*/
public B channelOptions(SocketOptions channelOptions) {
this.channelOptions = channelOptions;
return (B) this;
return identity();
}

/**
Expand All @@ -137,7 +139,7 @@ public B channelOptions(SocketOptions channelOptions) {
*/
public B dnsResolver(DnsResolver dnsResolver) {
this.dnsResolver = dnsResolver;
return (B) this;
return identity();
}

/**
Expand All @@ -148,7 +150,30 @@ public B dnsResolver(DnsResolver dnsResolver) {
*/
public B dnsAddressLookup(DnsAddressLookup dnsAddressLookup) {
this.dnsAddressLookup = dnsAddressLookup;
return (B) this;
return identity();
}

/**
* Whether to follow redirects.
*
* @param followRedirect whether to follow redirects
* @return updated builder
*/
public B followRedirect(boolean followRedirect) {
this.followRedirect = followRedirect;
return identity();
}

/**
* Max number of followed redirects.
* This is ignored if followRedirect option is false.
*
* @param maxRedirect max number of followed redirects
* @return updated builder
*/
public B maxRedirects(int maxRedirect) {
this.maxRedirect = maxRedirect;
return identity();
}

/**
Expand Down Expand Up @@ -186,5 +211,12 @@ DnsAddressLookup dnsAddressLookup() {
return dnsAddressLookup;
}

boolean followRedirect() {
return followRedirect;
}

int maxRedirect() {
return maxRedirect;
}
}
}

0 comments on commit d5de0d2

Please sign in to comment.