Fix issues with client prepending context path to relative redirects …

…and context paths not starting with /
micronaut-projects · Feb 12, 2020 · 30bd34d · 30bd34d
1 parent 1138f28
commit 30bd34d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 12 deletions.
diff --git a/http-client/src/main/java/io/micronaut/http/client/DefaultHttpClient.java b/http-client/src/main/java/io/micronaut/http/client/DefaultHttpClient.java
@@ -234,7 +234,14 @@ public DefaultHttpClient(@Parameter LoadBalancer loadBalancer,
 
         this.loadBalancer = loadBalancer;
         this.defaultCharset = configuration.getDefaultCharset();
-        this.contextPath = contextPath;
+        if (StringUtils.isNotEmpty(contextPath)) {
+            if (contextPath.charAt(0) != '/') {
+                contextPath = '/' + contextPath;
+            }
+            this.contextPath = contextPath;
+        } else {
+            this.contextPath = null;
+        }
         this.nettyClientSslBuilder = nettyClientSslBuilder;
         this.bootstrap = new Bootstrap();
         this.configuration = configuration;
@@ -724,6 +731,13 @@ protected void configure(BeanContext beanContext) {
         }
     }
 
+    private <I, O, E> Flowable<io.micronaut.http.HttpResponse<O>> redirectExchange(io.micronaut.http.HttpRequest<I> request, Argument<O> bodyType, Argument<E> errorType) {
+        final io.micronaut.http.HttpRequest<Object> parentRequest = ServerRequestContext.currentRequest().orElse(null);
+        Publisher<URI> uriPublisher = resolveRequestURI(request, false);
+        return Flowable.fromPublisher(uriPublisher)
+                .switchMap(buildExchangePublisher(parentRequest, request, bodyType, errorType));
+    }
+
     private <T> Flowable<T> connectWebSocket(URI uri, MutableHttpRequest<?> request, Class<T> clientEndpointType, WebSocketBean<T> webSocketBean) {
         Bootstrap bootstrap = this.bootstrap.clone();
         if (webSocketBean == null) {
@@ -1109,6 +1123,16 @@ protected void closeChannelAsync(Channel channel) {
      * @return A {@link Publisher} with the resolved URI
      */
     protected <I> Publisher<URI> resolveRequestURI(io.micronaut.http.HttpRequest<I> request) {
+        return resolveRequestURI(request, true);
+    }
+
+    /**
+     * @param request            The request
+     * @param includeContextPath Whether to prepend the client context path
+     * @param <I>                The input type
+     * @return A {@link Publisher} with the resolved URI
+     */
+    protected <I> Publisher<URI> resolveRequestURI(io.micronaut.http.HttpRequest<I> request, boolean includeContextPath) {
         URI requestURI;
         if (!request.getParameters().isEmpty()) {
             UriBuilder newUri = UriBuilder.of(request.getUri());
@@ -1131,7 +1155,7 @@ protected <I> Publisher<URI> resolveRequestURI(io.micronaut.http.HttpRequest<I>
                                 ((MutableHttpRequest) request).getHeaders().auth(authInfo.get());
                             }
                         }
-                        return server.resolve(resolveRequestURI(requestURI));
+                        return server.resolve(includeContextPath ? prependContextPath(requestURI) : requestURI);
                     }
             );
         }
@@ -1141,7 +1165,7 @@ protected <I> Publisher<URI> resolveRequestURI(io.micronaut.http.HttpRequest<I>
      * @param requestURI The request URI
      * @return A URI that is prepended with the contextPath, if set
      */
-    protected URI resolveRequestURI(URI requestURI) {
+    protected URI prependContextPath(URI requestURI) {
         if (StringUtils.isNotEmpty(contextPath)) {
             try {
                 return new URI(StringUtils.prependUri(contextPath, requestURI.toString()));
@@ -1601,7 +1625,7 @@ protected void channelRead0(ChannelHandlerContext ctx, StreamedHttpResponse msg)
                         try {
                             MutableHttpRequest<Object> redirectRequest = io.micronaut.http.HttpRequest.GET(location);
                             setRedirectHeaders(nettyRequest, redirectRequest);
-                            redirectedExchange = Flowable.fromPublisher(resolveRequestURI(redirectRequest))
+                            redirectedExchange = Flowable.fromPublisher(resolveRequestURI(redirectRequest, false))
                                     .flatMap(uri -> buildStreamExchange(parentRequest, redirectRequest, uri));
 
                             //noinspection SubscriberImplementation
@@ -1759,7 +1783,7 @@ protected void channelRead0(ChannelHandlerContext channelHandlerContext, FullHtt
                             String location = headers.get(HttpHeaderNames.LOCATION);
                             final MutableHttpRequest<Object> redirectRequest = io.micronaut.http.HttpRequest.GET(location);
                             setRedirectHeaders(request, redirectRequest);
-                            Flowable<io.micronaut.http.HttpResponse<O>> redirectExchange = exchange(redirectRequest, bodyType);
+                            Flowable<io.micronaut.http.HttpResponse<O>> redirectExchange = redirectExchange(redirectRequest, bodyType, DEFAULT_ERROR_TYPE);
                             redirectExchange.first(io.micronaut.http.HttpResponse.notFound())
                                     .subscribe((oHttpResponse, throwable) -> {
                                         if (throwable != null) {

diff --git a/http-client/src/test/groovy/io/micronaut/http/client/ClientRedirectSpec.groovy b/http-client/src/test/groovy/io/micronaut/http/client/ClientRedirectSpec.groovy
@@ -1,5 +1,6 @@
 package io.micronaut.http.client
 
+import groovy.transform.NotYetImplemented
 import io.micronaut.context.ApplicationContext
 import io.micronaut.http.HttpResponse
 import io.micronaut.http.HttpStatus
@@ -11,9 +12,6 @@ import spock.lang.AutoCleanup
 import spock.lang.Shared
 import spock.lang.Specification
 
-/**
- * Tests of Micronaut HTTP Client relative URIs and redirects.
- */
 class ClientRedirectSpec extends Specification {
 
     @Shared @AutoCleanup EmbeddedServer embeddedServer =
@@ -51,7 +49,8 @@ class ClientRedirectSpec extends Specification {
         client.close()
     }
 
-    void "BUG: test - client: full uri, redirect: relative"() {
+    @NotYetImplemented
+    void "test - client: full uri, redirect: relative"() {
         given:
         RxHttpClient client = RxHttpClient.create(embeddedServer.getURL())
 
@@ -64,7 +63,6 @@ class ClientRedirectSpec extends Specification {
         response.body() == "It works!"
 
         cleanup:
-        client.stop()
         client.close()
     }
 
@@ -84,7 +82,7 @@ class ClientRedirectSpec extends Specification {
         client.close()
     }
 
-    void "BUG: test - client: relative uri - no slash"() {
+    void "test - client: relative uri - no slash"() {
         given:
         RxHttpClient client = new DefaultHttpClient(embeddedServer.getURL(), new DefaultHttpClientConfiguration(), "test")
 
@@ -100,7 +98,7 @@ class ClientRedirectSpec extends Specification {
         client.close()
     }
 
-    void "BUG: test - client: relative uri, redirect: absolute "() {
+    void "test - client: relative uri, redirect: absolute "() {
         given:
         RxHttpClient client = new DefaultHttpClient(embeddedServer.getURL(), new DefaultHttpClientConfiguration(), "/test")