Skip to content

Níma WebClient redirect support #6929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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 @@ -232,6 +236,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")
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.FOUND_302));
}

try (Http1ClientResponse response = injectedHttp1client.put("/redirect")
.followRedirects(true)
.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")
.submit("Test entity")) {
assertThat(response.status(), is(Http.Status.TEMPORARY_REDIRECT_307));
}

try (Http1ClientResponse response = injectedHttp1client.put("/redirectKeepMethod")
.followRedirects(true)
.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
Expand Up @@ -66,4 +66,20 @@ public interface ClientConfig {
*/
DnsAddressLookup dnsAddressLookup();

/**
* Whether to follow redirects.
*
* @return follow redirects
*/
@ConfiguredOption("false")
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 @@ -109,6 +109,7 @@ public void scheme(String scheme) {
*/
public void host(String host) {
this.host = host;
authority(host, port);
}

/**
Expand All @@ -118,6 +119,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;
}
}
}
Loading