Commit 2588b11d by Johannes Edmeier

Fix proxying of requests where the path contains spaces

Setting a loglevel for logger names with spaces currently fails because the servlet.InstanceProxyController messes with the encoding. With this commit the path is correctly encoded before being forwarded. fixes #755
parent 436357c7
......@@ -77,7 +77,7 @@ public class AbstractInstancesProxyController {
HttpMethod method,
HttpHeaders headers,
Supplier<BodyInserter<?, ? super ClientHttpRequest>> bodyInserter) {
log.trace("Proxy-Request for instance {} / {}", instanceId, uri);
log.trace("Proxy-Request for instance {} with URL '{}'", instanceId, uri);
return registry.getInstance(InstanceId.of(instanceId))
.flatMap(instance -> forward(instance, uri, method, headers, bodyInserter))
......@@ -92,7 +92,8 @@ public class AbstractInstancesProxyController {
Supplier<BodyInserter<?, ? super ClientHttpRequest>> bodyInserter) {
WebClient.RequestBodySpec bodySpec = instanceWebClient.instance(instance)
.method(method)
.uri(uri).headers(h -> h.addAll(filterHeaders(headers)));
.uri(uri)
.headers(h -> h.addAll(filterHeaders(headers)));
WebClient.RequestHeadersSpec<?> headersSpec = bodySpec;
if (requiresBody(method)) {
......@@ -103,15 +104,19 @@ public class AbstractInstancesProxyController {
}
}
return headersSpec.exchange()
.timeout(this.readTimeout, Mono.fromSupplier(
() -> ClientResponse.create(HttpStatus.GATEWAY_TIMEOUT, strategies).build()))
.onErrorResume(ResolveEndpointException.class, ex -> Mono.fromSupplier(
() -> ClientResponse.create(HttpStatus.NOT_FOUND, strategies).build()))
.onErrorResume(IOException.class, ex -> Mono.fromSupplier(
() -> ClientResponse.create(HttpStatus.BAD_GATEWAY, strategies).build()))
.onErrorResume(ConnectException.class, ex -> Mono.fromSupplier(
() -> ClientResponse.create(HttpStatus.BAD_GATEWAY, strategies).build()));
return headersSpec.exchange().timeout(this.readTimeout, Mono.fromSupplier(() -> {
log.trace("Timeout for Proxy-Request for instance {} with URL '{}'", instance.getId(), uri);
return ClientResponse.create(HttpStatus.GATEWAY_TIMEOUT, strategies).build();
})).onErrorResume(ResolveEndpointException.class, ex -> Mono.fromSupplier(() -> {
log.trace("No Endpoint found for Proxy-Request for instance {} with URL '{}'", instance.getId(), uri);
return ClientResponse.create(HttpStatus.NOT_FOUND, strategies).build();
})).onErrorResume(IOException.class, ex -> Mono.fromSupplier(() -> {
log.trace("Error for Proxy-Request for instance {} with URL '{}' timed out", instance.getId(), uri, ex);
return ClientResponse.create(HttpStatus.BAD_GATEWAY, strategies).build();
})).onErrorResume(ConnectException.class, ex -> Mono.fromSupplier(() -> {
log.trace("Error for Proxy-Request for instance {} with URL '{}' timed out", instance.getId(), uri, ex);
return ClientResponse.create(HttpStatus.BAD_GATEWAY, strategies).build();
}));
}
protected String getEndpointLocalPath(String pathWithinApplication) {
......
......@@ -26,6 +26,8 @@ import reactor.core.publisher.Mono;
import java.net.URI;
import java.util.Optional;
import java.util.function.Function;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
......@@ -41,6 +43,7 @@ import static de.codecentric.boot.admin.server.utils.MediaType.ACTUATOR_V2_MEDIA
import static java.util.Collections.singletonList;
public final class InstanceExchangeFilterFunctions {
private static final Logger log = LoggerFactory.getLogger(InstanceExchangeFilterFunctions.class);
public static final String ATTRIBUTE_INSTANCE = "instance";
public static final String ATTRIBUTE_ENDPOINT = "endpointId";
......@@ -82,6 +85,7 @@ public final class InstanceExchangeFilterFunctions {
public static ExchangeFilterFunction rewriteEndpointUrl() {
return toExchangeFilterFunction((instance, request, next) -> {
if (request.url().isAbsolute()) {
log.trace("Absolute URL '{}' for instance {} not rewritten", request.url(), instance.getId());
return next.exchange(request);
}
......@@ -98,6 +102,8 @@ public final class InstanceExchangeFilterFunctions {
}
URI newUrl = rewriteUrl(oldUrl, endpoint.get().getUrl());
log.trace("URL '{}' for Endpoint {} of instance {} rewritten to {}", oldUrl, endpoint.get().getId(),
instance.getId(), newUrl);
ClientRequest newRequest = ClientRequest.from(request)
.attribute(ATTRIBUTE_ENDPOINT, endpoint.get().getId())
.url(newUrl)
......@@ -132,7 +138,8 @@ public final class InstanceExchangeFilterFunctions {
};
}
private static Function<ClientResponse, Mono<ClientResponse>> convertClientResponse(Function<Flux<DataBuffer>, Flux<DataBuffer>> bodConverter, MediaType contentType) {
private static Function<ClientResponse, Mono<ClientResponse>> convertClientResponse(Function<Flux<DataBuffer>, Flux<DataBuffer>> bodConverter,
MediaType contentType) {
return response -> {
ClientResponse convertedResponse = ClientResponse.from(response).headers(headers -> {
headers.replace(HttpHeaders.CONTENT_TYPE, singletonList(contentType.toString()));
......
......@@ -71,8 +71,8 @@ public class InstancesProxyController extends AbstractInstancesProxyController {
ServerHttpRequest request = new ServletServerHttpRequest(servletRequest);
ServerHttpResponse response = new ServletServerHttpResponse(servletResponse);
String pathWithinApplication = servletRequest.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE)
.toString();
String pathWithinApplication = UriComponentsBuilder.fromPath(
servletRequest.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).toString()).toUriString();
String endpointLocalPath = getEndpointLocalPath(pathWithinApplication);
URI uri = UriComponentsBuilder.fromPath(endpointLocalPath)
......
......@@ -40,6 +40,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
import static com.github.tomakehurst.wiremock.client.WireMock.delete;
import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
import static com.github.tomakehurst.wiremock.client.WireMock.ok;
import static com.github.tomakehurst.wiremock.client.WireMock.post;
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
......@@ -119,6 +120,26 @@ public abstract class AbstractInstancesProxyControllerIntegrationTest {
}
@Test
public void should_forward_requests_with_spaces_in_path() {
String instanceId = registerInstance("http://localhost:" + wireMock.port() + "/mgmt");
wireMock.stubFor(get(urlEqualTo("/mgmt/test/has%20spaces")).willReturn(
ok("{ \"foo\" : \"bar\" }").withHeader(CONTENT_TYPE, ActuatorMediaType.V2_JSON + ";charset=UTF-8")));
client.get()
.uri("/instances/{instanceId}/actuator/test/has spaces", instanceId)
.accept(ACTUATOR_V2_MEDIATYPE)
.exchange()
.expectStatus()
.isEqualTo(HttpStatus.OK)
.expectBody()
.jsonPath("$.foo")
.isEqualTo("bar");
wireMock.verify(getRequestedFor(urlEqualTo("/mgmt/test/has%20spaces")));
}
@Test
public void should_forward_requests() {
String instanceId = registerInstance("http://localhost:" + wireMock.port() + "/mgmt");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment