From 67c68ef8ba3458456b8db74b5d51530ed9f363ea Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 21 Oct 2014 09:47:42 +0200 Subject: [PATCH] CorsHandler should release HttpRequest after processing preflight/error. Motivation: Currently, when the CorsHandler processes a preflight request, or respondes with an 403 Forbidden using the short-curcuit option, the HttpRequest is not released which leads to a buffer leak. Modifications: Releasing the HttpRequest when done processing a preflight request or responding with an 403. Result: Using the CorsHandler will not cause buffer leaks. --- .../handler/codec/http/cors/CorsHandler.java | 3 ++ .../codec/http/cors/CorsHandlerTest.java | 29 +++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java index dea12c83ce..56fb81e820 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/cors/CorsHandler.java @@ -29,6 +29,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import static io.netty.handler.codec.http.HttpHeaders.Names.*; import static io.netty.handler.codec.http.HttpMethod.*; import static io.netty.handler.codec.http.HttpResponseStatus.*; +import static io.netty.util.ReferenceCountUtil.release; /** * Handles Cross Origin Resource Sharing (CORS) requests. @@ -72,6 +73,7 @@ public class CorsHandler extends ChannelDuplexHandler { setMaxAge(response); setPreflightHeaders(response); } + release(request); ctx.writeAndFlush(response).addListener(ChannelFutureListener.CLOSE); } @@ -199,6 +201,7 @@ public class CorsHandler extends ChannelDuplexHandler { private static void forbidden(final ChannelHandlerContext ctx, final HttpRequest request) { ctx.writeAndFlush(new DefaultFullHttpResponse(request.protocolVersion(), FORBIDDEN)) .addListener(ChannelFutureListener.CLOSE); + release(request); } } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java index de2c611882..8c07132b66 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/cors/CorsHandlerTest.java @@ -237,6 +237,27 @@ public class CorsHandlerTest { assertThat(response.headers().get(ACCESS_CONTROL_ALLOW_ORIGIN), is(nullValue())); } + @Test + public void preflightRequestShouldReleaseRequest() { + final CorsConfig config = CorsConfig.withOrigin("http://localhost:8888") + .preflightResponseHeader("CustomHeader", Arrays.asList("value1", "value2")) + .build(); + final EmbeddedChannel channel = new EmbeddedChannel(new CorsHandler(config)); + final FullHttpRequest request = optionsRequest("http://localhost:8888", "content-type, xheader1"); + channel.writeInbound(request); + assertThat(request.refCnt(), is(0)); + } + + @Test + public void forbiddenShouldReleaseRequest() { + final CorsConfig config = CorsConfig.withOrigin("https://localhost").shortCurcuit().build(); + final EmbeddedChannel channel = new EmbeddedChannel(new CorsHandler(config), new EchoHandler()); + final FullHttpRequest request = createHttpRequest(GET); + request.headers().set(ORIGIN, "http://localhost:8888"); + channel.writeInbound(request); + assertThat(request.refCnt(), is(0)); + } + private static HttpResponse simpleRequest(final CorsConfig config, final String origin) { return simpleRequest(config, origin, null); } @@ -267,12 +288,16 @@ public class CorsHandlerTest { final String origin, final String requestHeaders) { final EmbeddedChannel channel = new EmbeddedChannel(new CorsHandler(config)); + channel.writeInbound(optionsRequest(origin, requestHeaders)); + return (HttpResponse) channel.readOutbound(); + } + + private static FullHttpRequest optionsRequest(final String origin, final String requestHeaders) { final FullHttpRequest httpRequest = createHttpRequest(OPTIONS); httpRequest.headers().set(ORIGIN, origin); httpRequest.headers().set(ACCESS_CONTROL_REQUEST_METHOD, httpRequest.method().toString()); httpRequest.headers().set(ACCESS_CONTROL_REQUEST_HEADERS, requestHeaders); - channel.writeInbound(httpRequest); - return (HttpResponse) channel.readOutbound(); + return httpRequest; } private static FullHttpRequest createHttpRequest(HttpMethod method) {