Fix URI parsing in Reactor Netty request
Issue: SPR-15560
This commit is contained in:
		
							parent
							
								
									c59e192b0f
								
							
						
					
					
						commit
						11075f12bc
					
				|  | @ -16,6 +16,7 @@ | ||||||
| 
 | 
 | ||||||
| package org.springframework.http.server.reactive; | package org.springframework.http.server.reactive; | ||||||
| 
 | 
 | ||||||
|  | import java.net.URISyntaxException; | ||||||
| import java.util.function.BiFunction; | import java.util.function.BiFunction; | ||||||
| 
 | 
 | ||||||
| import io.netty.handler.codec.http.HttpResponseStatus; | import io.netty.handler.codec.http.HttpResponseStatus; | ||||||
|  | @ -53,10 +54,19 @@ public class ReactorHttpHandlerAdapter | ||||||
| 	public Mono<Void> apply(HttpServerRequest request, HttpServerResponse response) { | 	public Mono<Void> apply(HttpServerRequest request, HttpServerResponse response) { | ||||||
| 
 | 
 | ||||||
| 		NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(response.alloc()); | 		NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(response.alloc()); | ||||||
| 		ReactorServerHttpRequest req = new ReactorServerHttpRequest(request, bufferFactory); | 		ReactorServerHttpRequest adaptedRequest; | ||||||
| 		ReactorServerHttpResponse resp = new ReactorServerHttpResponse(response, bufferFactory); | 		ReactorServerHttpResponse adaptedResponse; | ||||||
|  | 		try { | ||||||
|  | 			adaptedRequest = new ReactorServerHttpRequest(request, bufferFactory); | ||||||
|  | 			adaptedResponse = new ReactorServerHttpResponse(response, bufferFactory); | ||||||
|  | 		} | ||||||
|  | 		catch (URISyntaxException ex) { | ||||||
|  | 			logger.error("Invalid URL " + ex.getMessage(), ex); | ||||||
|  | 			response.status(HttpResponseStatus.BAD_REQUEST); | ||||||
|  | 			return Mono.empty(); | ||||||
|  | 		} | ||||||
| 
 | 
 | ||||||
| 		return this.httpHandler.handle(req, resp) | 		return this.httpHandler.handle(adaptedRequest, adaptedResponse) | ||||||
| 				.onErrorResume(ex -> { | 				.onErrorResume(ex -> { | ||||||
| 					logger.error("Could not complete request", ex); | 					logger.error("Could not complete request", ex); | ||||||
| 					response.status(HttpResponseStatus.INTERNAL_SERVER_ERROR); | 					response.status(HttpResponseStatus.INTERNAL_SERVER_ERROR); | ||||||
|  |  | ||||||
|  | @ -48,28 +48,25 @@ public class ReactorServerHttpRequest extends AbstractServerHttpRequest { | ||||||
| 	private final NettyDataBufferFactory bufferFactory; | 	private final NettyDataBufferFactory bufferFactory; | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| 	public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactory bufferFactory) { | 	public ReactorServerHttpRequest(HttpServerRequest request, NettyDataBufferFactory bufferFactory) | ||||||
|  | 			throws URISyntaxException { | ||||||
|  | 
 | ||||||
| 		super(initUri(request), initHeaders(request)); | 		super(initUri(request), initHeaders(request)); | ||||||
| 		Assert.notNull(bufferFactory, "'bufferFactory' must not be null"); | 		Assert.notNull(bufferFactory, "'bufferFactory' must not be null"); | ||||||
| 		this.request = request; | 		this.request = request; | ||||||
| 		this.bufferFactory = bufferFactory; | 		this.bufferFactory = bufferFactory; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static URI initUri(HttpServerRequest channel) { | 	private static URI initUri(HttpServerRequest channel) throws URISyntaxException { | ||||||
| 		Assert.notNull(channel, "'channel' must not be null"); | 		Assert.notNull(channel, "'channel' must not be null"); | ||||||
| 		InetSocketAddress address = channel.remoteAddress(); | 		InetSocketAddress address = channel.remoteAddress(); | ||||||
| 		String requestUri = channel.uri(); | 		String requestUri = channel.uri(); | ||||||
| 		return (address != null ? getBaseUrl(address).resolve(requestUri) : URI.create(requestUri)); | 		return (address != null ? createUrl(address, requestUri) : new URI(requestUri)); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static URI getBaseUrl(InetSocketAddress address) { | 	private static URI createUrl(InetSocketAddress address, String requestUri) throws URISyntaxException { | ||||||
| 		try { | 		URI baseUrl = new URI(null, null, address.getHostString(), address.getPort(), null, null, null); | ||||||
| 			return new URI(null, null, address.getHostString(), address.getPort(), null, null, null); | 		return new URI(baseUrl.toString() + requestUri); | ||||||
| 		} |  | ||||||
| 		catch (URISyntaxException ex) { |  | ||||||
| 			// Should not happen... |  | ||||||
| 			throw new IllegalStateException(ex); |  | ||||||
| 		} |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static HttpHeaders initHeaders(HttpServerRequest channel) { | 	private static HttpHeaders initHeaders(HttpServerRequest channel) { | ||||||
|  |  | ||||||
|  | @ -16,7 +16,6 @@ | ||||||
| 
 | 
 | ||||||
| package org.springframework.http.server.reactive; | package org.springframework.http.server.reactive; | ||||||
| 
 | 
 | ||||||
| import java.io.IOException; |  | ||||||
| import java.net.URI; | import java.net.URI; | ||||||
| 
 | 
 | ||||||
| import org.junit.Test; | import org.junit.Test; | ||||||
|  | @ -45,45 +44,55 @@ public class ErrorHandlerIntegrationTests extends AbstractHttpHandlerIntegration | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Test | 	@Test | ||||||
| 	public void response() throws Exception { | 	public void responseBodyError() throws Exception { | ||||||
| 		// TODO: fix Reactor | 		// TODO: fix Reactor | ||||||
| 		assumeFalse(server instanceof ReactorHttpServer); | 		assumeFalse(server instanceof ReactorHttpServer); | ||||||
| 
 | 
 | ||||||
| 		RestTemplate restTemplate = new RestTemplate(); | 		RestTemplate restTemplate = new RestTemplate(); | ||||||
| 		restTemplate.setErrorHandler(NO_OP_ERROR_HANDLER); | 		restTemplate.setErrorHandler(NO_OP_ERROR_HANDLER); | ||||||
| 
 | 
 | ||||||
| 		ResponseEntity<String> response = restTemplate | 		URI url = new URI("http://localhost:" + port + "/response-body-error"); | ||||||
| 				.getForEntity(new URI("http://localhost:" + port + "/response"), | 		ResponseEntity<String> response = restTemplate.getForEntity(url, String.class); | ||||||
| 						String.class); |  | ||||||
| 
 | 
 | ||||||
| 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); | 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Test | 	@Test | ||||||
| 	public void returnValue() throws Exception { | 	public void handlingError() throws Exception { | ||||||
| 		// TODO: fix Reactor | 		// TODO: fix Reactor | ||||||
| 		assumeFalse(server instanceof ReactorHttpServer); | 		assumeFalse(server instanceof ReactorHttpServer); | ||||||
| 
 | 
 | ||||||
| 		RestTemplate restTemplate = new RestTemplate(); | 		RestTemplate restTemplate = new RestTemplate(); | ||||||
| 		restTemplate.setErrorHandler(NO_OP_ERROR_HANDLER); | 		restTemplate.setErrorHandler(NO_OP_ERROR_HANDLER); | ||||||
| 
 | 
 | ||||||
| 		ResponseEntity<String> response = restTemplate | 		URI url = new URI("http://localhost:" + port + "/handling-error"); | ||||||
| 				.getForEntity(new URI("http://localhost:" + port + "/returnValue"), | 		ResponseEntity<String> response = restTemplate.getForEntity(url, String.class); | ||||||
| 						String.class); |  | ||||||
| 
 | 
 | ||||||
| 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); | 		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	@Test // SPR-15560 | ||||||
|  | 	public void emptyPathSegments() throws Exception { | ||||||
|  | 
 | ||||||
|  | 		RestTemplate restTemplate = new RestTemplate(); | ||||||
|  | 		restTemplate.setErrorHandler(NO_OP_ERROR_HANDLER); | ||||||
|  | 
 | ||||||
|  | 		URI url = new URI("http://localhost:" + port + "//"); | ||||||
|  | 		ResponseEntity<String> response = restTemplate.getForEntity(url, String.class); | ||||||
|  | 
 | ||||||
|  | 		assertEquals(HttpStatus.OK, response.getStatusCode()); | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	private static class ErrorHandler implements HttpHandler { | 	private static class ErrorHandler implements HttpHandler { | ||||||
| 
 | 
 | ||||||
| 		@Override | 		@Override | ||||||
| 		public Mono<Void> handle(ServerHttpRequest request, ServerHttpResponse response) { | 		public Mono<Void> handle(ServerHttpRequest request, ServerHttpResponse response) { | ||||||
| 			Exception error = new UnsupportedOperationException(); | 			Exception error = new UnsupportedOperationException(); | ||||||
| 			String path = request.getURI().getPath(); | 			String path = request.getURI().getPath(); | ||||||
| 			if (path.endsWith("response")) { | 			if (path.endsWith("response-body-error")) { | ||||||
| 				return response.writeWith(Mono.error(error)); | 				return response.writeWith(Mono.error(error)); | ||||||
| 			} | 			} | ||||||
| 			else if (path.endsWith("returnValue")) { | 			else if (path.endsWith("handling-error")) { | ||||||
| 				return Mono.error(error); | 				return Mono.error(error); | ||||||
| 			} | 			} | ||||||
| 			else { | 			else { | ||||||
|  | @ -92,17 +101,16 @@ public class ErrorHandlerIntegrationTests extends AbstractHttpHandlerIntegration | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static final ResponseErrorHandler NO_OP_ERROR_HANDLER = | 	private static final ResponseErrorHandler NO_OP_ERROR_HANDLER = new ResponseErrorHandler() { | ||||||
| 			new ResponseErrorHandler() { |  | ||||||
| 
 | 
 | ||||||
| 				@Override | 		@Override | ||||||
| 				public boolean hasError(ClientHttpResponse response) throws IOException { | 		public boolean hasError(ClientHttpResponse response) { | ||||||
| 					return false; | 			return false; | ||||||
| 				} | 		} | ||||||
| 
 | 
 | ||||||
| 				@Override | 		@Override | ||||||
| 				public void handleError(ClientHttpResponse response) throws IOException { | 		public void handleError(ClientHttpResponse response) { | ||||||
| 				} | 		} | ||||||
| 			}; | 	}; | ||||||
| 
 | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -17,6 +17,7 @@ | ||||||
| package org.springframework.http.server.reactive; | package org.springframework.http.server.reactive; | ||||||
| 
 | 
 | ||||||
| import java.net.InetSocketAddress; | import java.net.InetSocketAddress; | ||||||
|  | import java.net.URISyntaxException; | ||||||
| 
 | 
 | ||||||
| import org.apache.commons.logging.Log; | import org.apache.commons.logging.Log; | ||||||
| import org.apache.commons.logging.LogFactory; | import org.apache.commons.logging.LogFactory; | ||||||
|  | @ -63,8 +64,17 @@ public class RxNettyHttpHandlerAdapter implements RequestHandler<ByteBuf, ByteBu | ||||||
| 		NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(channel.alloc()); | 		NettyDataBufferFactory bufferFactory = new NettyDataBufferFactory(channel.alloc()); | ||||||
| 		InetSocketAddress remoteAddress = (InetSocketAddress) channel.remoteAddress(); | 		InetSocketAddress remoteAddress = (InetSocketAddress) channel.remoteAddress(); | ||||||
| 
 | 
 | ||||||
| 		RxNettyServerHttpRequest request = new RxNettyServerHttpRequest(nativeRequest, bufferFactory, remoteAddress); | 		RxNettyServerHttpRequest request; | ||||||
| 		RxNettyServerHttpResponse response = new RxNettyServerHttpResponse(nativeResponse, bufferFactory); | 		RxNettyServerHttpResponse response; | ||||||
|  | 		try { | ||||||
|  | 			request = new RxNettyServerHttpRequest(nativeRequest, bufferFactory, remoteAddress); | ||||||
|  | 			response = new RxNettyServerHttpResponse(nativeResponse, bufferFactory); | ||||||
|  | 		} | ||||||
|  | 		catch (URISyntaxException ex) { | ||||||
|  | 			logger.error("Could not complete request", ex); | ||||||
|  | 			nativeResponse.setStatus(HttpResponseStatus.BAD_REQUEST); | ||||||
|  | 			return Observable.empty(); | ||||||
|  | 		} | ||||||
| 
 | 
 | ||||||
| 		Publisher<Void> result = this.httpHandler.handle(request, response) | 		Publisher<Void> result = this.httpHandler.handle(request, response) | ||||||
| 				.onErrorResume(ex -> { | 				.onErrorResume(ex -> { | ||||||
|  |  | ||||||
|  | @ -55,7 +55,8 @@ public class RxNettyServerHttpRequest extends AbstractServerHttpRequest { | ||||||
| 
 | 
 | ||||||
| 
 | 
 | ||||||
| 	public RxNettyServerHttpRequest(HttpServerRequest<ByteBuf> request, | 	public RxNettyServerHttpRequest(HttpServerRequest<ByteBuf> request, | ||||||
| 			NettyDataBufferFactory dataBufferFactory, InetSocketAddress remoteAddress) { | 			NettyDataBufferFactory dataBufferFactory, InetSocketAddress remoteAddress) | ||||||
|  | 			throws URISyntaxException { | ||||||
| 
 | 
 | ||||||
| 		super(initUri(request, remoteAddress), initHeaders(request)); | 		super(initUri(request, remoteAddress), initHeaders(request)); | ||||||
| 
 | 
 | ||||||
|  | @ -65,20 +66,17 @@ public class RxNettyServerHttpRequest extends AbstractServerHttpRequest { | ||||||
| 		this.remoteAddress = remoteAddress; | 		this.remoteAddress = remoteAddress; | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static URI initUri(HttpServerRequest<ByteBuf> request, InetSocketAddress remoteAddress) { | 	private static URI initUri(HttpServerRequest<ByteBuf> request, InetSocketAddress remoteAddress) | ||||||
|  | 			throws URISyntaxException { | ||||||
|  | 
 | ||||||
| 		Assert.notNull(request, "'request' must not be null"); | 		Assert.notNull(request, "'request' must not be null"); | ||||||
| 		String requestUri = request.getUri(); | 		String requestUri = request.getUri(); | ||||||
| 		return remoteAddress != null ? getBaseUrl(remoteAddress).resolve(requestUri) : URI.create(requestUri); | 		return remoteAddress != null ? createUrl(remoteAddress, requestUri) : URI.create(requestUri); | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static URI getBaseUrl(InetSocketAddress address) { | 	private static URI createUrl(InetSocketAddress address, String requestUri) throws URISyntaxException { | ||||||
| 		try { | 		URI baseUrl = new URI(null, null, address.getHostString(), address.getPort(), null, null, null); | ||||||
| 			return new URI(null, null, address.getHostString(), address.getPort(), null, null, null); | 		return new URI(baseUrl.toString() + requestUri); | ||||||
| 		} |  | ||||||
| 		catch (URISyntaxException ex) { |  | ||||||
| 			// Should not happen... |  | ||||||
| 			throw new IllegalStateException(ex); |  | ||||||
| 		} |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	private static HttpHeaders initHeaders(HttpServerRequest<ByteBuf> request) { | 	private static HttpHeaders initHeaders(HttpServerRequest<ByteBuf> request) { | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue