Skip to content
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

fix #235: Allow setting followRedirect per HttpClient #260

Merged
merged 1 commit into from
Jan 16, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
35 changes: 30 additions & 5 deletions src/main/java/reactor/ipc/netty/http/client/HttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import io.netty.bootstrap.Bootstrap;
import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.handler.codec.http.HttpHeaderValues;
import io.netty.handler.codec.http.HttpHeaders;
Expand All @@ -33,7 +32,6 @@
import reactor.ipc.netty.NettyOutbound;
import reactor.ipc.netty.channel.BootstrapHandlers;
import reactor.ipc.netty.channel.ChannelOperations;
import reactor.ipc.netty.http.HttpResources;
import reactor.ipc.netty.resources.PoolResources;
import reactor.ipc.netty.tcp.TcpClient;
import reactor.ipc.netty.tcp.TcpServer;
Expand Down Expand Up @@ -247,6 +245,15 @@ public final HttpClient compress() {
return tcpConfiguration(COMPRESS_ATTR_CONFIG).headers(COMPRESS_HEADERS);
}

/**
* Enable http status 301/302 auto-redirect support
*
* @return a new {@link HttpClient}
*/
public final HttpClient followRedirect() {
return tcpConfiguration(FOLLOW_REDIRECT_ATTR_CONFIG);
}

/**
* HTTP DELETE to connect the {@link HttpClient}.
*
Expand Down Expand Up @@ -340,6 +347,15 @@ public final HttpClient noCompression() {
return tcpConfiguration(COMPRESS_ATTR_DISABLE).headers(COMPRESS_HEADERS_DISABLE);
}

/**
* Disable http status 301/302 auto-redirect support
*
* @return a new {@link HttpClient}
*/
public final HttpClient noRedirection() {
return tcpConfiguration(FOLLOW_REDIRECT_ATTR_DISABLE);
}

/**
* HTTP OPTIONS to connect the {@link HttpClient}.
*
Expand Down Expand Up @@ -505,15 +521,24 @@ static String reactorNettyVersion() {

static final LoggingHandler LOGGING_HANDLER = new LoggingHandler(HttpClient.class);

static final AttributeKey<Boolean> ACCEPT_GZIP =
static final AttributeKey<Boolean> ACCEPT_GZIP =
AttributeKey.newInstance("acceptGzip");

static final Function<TcpClient, TcpClient> COMPRESS_ATTR_CONFIG =
static final AttributeKey<Boolean> FOLLOW_REDIRECT =
AttributeKey.newInstance("followRedirect");

static final Function<TcpClient, TcpClient> COMPRESS_ATTR_CONFIG =
tcp -> tcp.attr(ACCEPT_GZIP, true);

static final Function<TcpClient, TcpClient> COMPRESS_ATTR_DISABLE =
static final Function<TcpClient, TcpClient> COMPRESS_ATTR_DISABLE =
tcp -> tcp.attr(ACCEPT_GZIP, null);

static final Function<TcpClient, TcpClient> FOLLOW_REDIRECT_ATTR_CONFIG =
tcp -> tcp.attr(FOLLOW_REDIRECT, true);

static final Function<TcpClient, TcpClient> FOLLOW_REDIRECT_ATTR_DISABLE =
tcp -> tcp.attr(FOLLOW_REDIRECT, null);

static final Consumer<? super HttpHeaders> COMPRESS_HEADERS = h ->
h.add(HttpHeaderNames.ACCEPT_ENCODING, HttpHeaderValues.GZIP);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ protected Mono<? extends Connection> connect(TcpClient delegate) {

boolean compress = attrs.get(ACCEPT_GZIP) != null && (Boolean) attrs.get(ACCEPT_GZIP);

boolean followRedirect = attrs.get(FOLLOW_REDIRECT) != null && (Boolean) attrs.get(FOLLOW_REDIRECT);


String uri = (String)attrs.get(URI);

Expand All @@ -141,11 +143,12 @@ protected Mono<? extends Connection> connect(TcpClient delegate) {
delegate.isSecure());

HttpClientHandler handler =
new HttpClientHandler(attrs, reconnectableUriEndpoint, compress);
new HttpClientHandler(attrs, reconnectableUriEndpoint, compress, followRedirect);

reconnectableUriEndpoint.init(uri, handler.method);

b.attr(ACCEPT_GZIP, null)
.attr(FOLLOW_REDIRECT, null)
.attr(BODY, null)
.attr(HEADERS, null)
.attr(URI, null)
Expand Down Expand Up @@ -206,9 +209,11 @@ static final class HttpClientHandler
final HttpHeaders
defaultHeaders;
final BiFunction<? super HttpClientRequest, ? super NettyOutbound, ? extends NettyOutbound>handler;
final boolean followRedirect;

@SuppressWarnings("unchecked")
HttpClientHandler(Map<AttributeKey<?>, Object> attrs, ReconnectableUriEndpoint bridge, boolean compress) {
HttpClientHandler(Map<AttributeKey<?>, Object> attrs, ReconnectableUriEndpoint bridge,
boolean compress, boolean followRedirect) {
this.method = (HttpMethod) attrs.get(METHOD);
HttpHeaders defaultHeaders = (HttpHeaders) attrs.get(HttpClientConnect.HEADERS);

Expand All @@ -227,6 +232,7 @@ static final class HttpClientHandler
this.defaultHeaders = defaultHeaders;
this.handler = requestHandler;
this.bridge = bridge;
this.followRedirect = followRedirect;
}

@Override
Expand All @@ -242,6 +248,7 @@ public Publisher<Void> apply(NettyInbound in, NettyOutbound out) {
.set(HttpHeaderNames.HOST,
resolveHostHeaderValue(ch.address()))
.set(HttpHeaderNames.ACCEPT, ALL);
ch.followRedirect(followRedirect);

if (method == HttpMethod.GET || method == HttpMethod.HEAD || method == HttpMethod.DELETE) {
ch.chunkedTransfer(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,8 @@ public Map<CharSequence, Set<Cookie>> cookies() {
return Collections.emptyMap();
}

@Override
public HttpClientRequest followRedirect() {
redirectable = true;
return this;
public void followRedirect(boolean redirectable) {
this.redirectable = redirectable;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ default HttpClientRequest options(Consumer<? super NettyPipeline.SendOptions> co
return this;
}

/**
* Enable http status 302 auto-redirect support
*
* @return {@literal this}
*/
HttpClientRequest followRedirect();

/**
* Return true if headers and status have been sent to the client
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void testConstructorWithProvidedReplacement() {

HttpClientOperations ops1 = new HttpClientOperations(() -> channel,
ConnectionEvents.emptyListener());
ops1.followRedirect();
ops1.followRedirect(true);

HttpClientOperations ops2 = new HttpClientOperations(ops1);

Expand Down
31 changes: 13 additions & 18 deletions src/test/java/reactor/ipc/netty/http/client/HttpClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,9 @@ public void proxy() throws Exception {
.host("127.0.0.1")
.port(8888)))
.wiretap()
.request(HttpMethod.GET)
.followRedirect()
.get()
.uri("https://projectreactor.io")
.send((req, out) -> req.followRedirect()
.sendHeaders())
.responseContent()
.retain()
.asString()
Expand All @@ -318,19 +317,17 @@ public void nonProxyHosts() throws Exception {
.port(8888)
.nonProxyHosts("spring.io")))
.wiretap();
Mono<String> remote1 = client.request(HttpMethod.GET)
Mono<String> remote1 = client.followRedirect()
.get()
.uri("https://projectreactor.io")
.send((c, out) -> c.followRedirect()
.sendHeaders())
.responseContent()
.retain()
.asString()
.limitRate(1)
.reduce(String::concat);
Mono<String> remote2 = client.request(HttpMethod.GET)
Mono<String> remote2 = client.followRedirect()
.get()
.uri("https://spring.io")
.send((c, out) -> c.followRedirect()
.sendHeaders())
.responseContent()
.retain()
.asString()
Expand Down Expand Up @@ -371,10 +368,9 @@ public void postUpload() throws Exception {
res = HttpClient.prepare()
.tcpConfiguration(tcpClient -> tcpClient.host("google.com"))
.wiretap()
.request(HttpMethod.GET)
.followRedirect()
.get()
.uri("/search")
.send((c, out) -> c.followRedirect()
.sendHeaders())
.responseSingle((r, out) -> Mono.just(r.status().code()))
.log()
.block(Duration.ofSeconds(30));
Expand All @@ -401,10 +397,9 @@ public void simpleTest404_1() {
}

private void doSimpleTest404(HttpClient client) {
int res = client.request(HttpMethod.GET)
int res = client.followRedirect()
.get()
.uri("/unsupportedURI")
.send((c, out) -> c.followRedirect()
.sendHeaders())
.responseSingle((r, buf) -> Mono.just(r.status().code()))
.log()
.block(Duration.ofSeconds(30));
Expand Down Expand Up @@ -586,9 +581,9 @@ public void gzip() {
.wiretap()
.headers(h -> h.add("Accept-Encoding", "gzip")
.add("Accept-Encoding", "deflate"))
.request(HttpMethod.GET)
.followRedirect()
.get()
.uri("http://www.httpwatch.com")
.send((req, out) -> req.followRedirect().sendHeaders())
.response((r, buf) -> buf.asString()
.elementAt(0)
.map(s -> s.substring(0, Math.min(s.length() -1, 100)))
Expand All @@ -607,13 +602,13 @@ public void gzip() {
StepVerifier.create(
HttpClient.prepare()
.wiretap()
.followRedirect()
.request(HttpMethod.GET)
.uri("http://www.httpwatch.com")
.send((req, out) ->
req.withConnection(c -> c.addHandlerFirst
("gzipDecompressor", new
HttpContentDecompressor()))
.followRedirect()
.addHeader
("Accept-Encoding", "gzip")
.addHeader("Accept-Encoding", "deflate")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ private void redirectTests(String url) {

try {
Flux.range(0, this.numberOfTests)
.concatMap(i -> client.post()
.concatMap(i -> client.followRedirect()
.post()
.uri("/login")
.send((r, out) -> r.followRedirect())
.responseContent()
.then())
.blockLast(Duration.ofSeconds(30));
Expand Down Expand Up @@ -109,9 +109,9 @@ public void testIssue253() {
.wiretap();

String value =
client.request(HttpMethod.GET)
client.followRedirect()
.get()
.uri("/1")
.send((req, out) -> req.followRedirect().sendHeaders())
.responseContent()
.aggregate()
.asString()
Expand All @@ -126,9 +126,9 @@ public void testIssue253() {
.block(Duration.ofSeconds(30));
Assertions.assertThat(value).isNull();

value = client.request(HttpMethod.GET)
value = client.followRedirect()
.get()
.uri("/2")
.send((req, out) -> req.followRedirect().sendHeaders())
.responseContent()
.aggregate()
.asString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -957,9 +957,9 @@ public void redirectTests(String url) {

try {
Flux.range(0, this.numberOfTests)
.concatMap(i -> client.post()
.concatMap(i -> client.followRedirect()
.post()
.uri("/login")
.send((r, out) -> r.followRedirect())
.responseContent()
.log("reactor.req."+i)
.then())
Expand Down