From 64bbaeb7cdf67799e1962475534f889f958edea3 Mon Sep 17 00:00:00 2001 From: Flavia Rainone Date: Sat, 15 Feb 2020 05:40:11 -0300 Subject: [PATCH 1/4] [UNDERTOW-1657] Part 1 of fix authored by Stuart for the buffer leak when handling Expect:100-continue requests --- .../handlers/HttpContinueReadHandler.java | 5 ++ .../protocol/http/HttpReadListener.java | 4 + .../protocol/http/HttpTransferEncoding.java | 7 ++ ...duitWrappingHandlerBufferLeakTestCase.java | 83 +++++++++++++++++++ 4 files changed, 99 insertions(+) create mode 100644 core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java diff --git a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java index 33c5c25b03..2c482d7979 100644 --- a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java +++ b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java @@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit; import io.undertow.server.ConduitWrapper; +import io.undertow.server.Connectors; import io.undertow.server.protocol.http.HttpContinue; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; @@ -81,6 +82,7 @@ protected ContinueConduit(final StreamSourceConduit next, final HttpServerExchan public long transferTo(final long position, final long count, final FileChannel target) throws IOException { if (exchange.getStatusCode() == StatusCodes.EXPECTATION_FAILED) { //rejected + Connectors.terminateRequest(exchange); return -1; } if (!sent) { @@ -100,6 +102,7 @@ public long transferTo(final long position, final long count, final FileChannel public long transferTo(final long count, final ByteBuffer throughBuffer, final StreamSinkChannel target) throws IOException { if (exchange.getStatusCode() == StatusCodes.EXPECTATION_FAILED) { //rejected + Connectors.terminateRequest(exchange); return -1; } if (!sent) { @@ -119,6 +122,7 @@ public long transferTo(final long count, final ByteBuffer throughBuffer, final S public int read(final ByteBuffer dst) throws IOException { if (exchange.getStatusCode() == StatusCodes.EXPECTATION_FAILED) { //rejected + Connectors.terminateRequest(exchange); return -1; } if (!sent) { @@ -138,6 +142,7 @@ public int read(final ByteBuffer dst) throws IOException { public long read(final ByteBuffer[] dsts, final int offs, final int len) throws IOException { if (exchange.getStatusCode() == StatusCodes.EXPECTATION_FAILED) { //rejected + Connectors.terminateRequest(exchange); return -1; } if (!sent) { diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java b/core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java index c070361a29..ed04da9a56 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpReadListener.java @@ -365,6 +365,10 @@ public void exchangeComplete(final HttpServerExchange exchange) { } } } else if (!exchange.isPersistent()) { + if (connection.getExtraBytes() != null) { + connection.getExtraBytes().close(); + connection.setExtraBytes(null); + } ConnectionUtils.cleanClose(connection.getChannel(), connection); } else { //upgrade or connect handling diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpTransferEncoding.java b/core/src/main/java/io/undertow/server/protocol/http/HttpTransferEncoding.java index fe18b5f919..3eb4f1a73d 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpTransferEncoding.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpTransferEncoding.java @@ -34,6 +34,8 @@ import io.undertow.util.Headers; import io.undertow.util.HttpString; import io.undertow.util.Methods; +import io.undertow.util.StatusCodes; + import org.jboss.logging.Logger; import org.xnio.conduits.ConduitStreamSourceChannel; import org.xnio.conduits.StreamSinkConduit; @@ -224,6 +226,11 @@ static StreamSinkConduit createSinkConduit(final HttpServerExchange exchange) { final HeaderMap responseHeaders = exchange.getResponseHeaders(); // test to see if we're still persistent String connection = responseHeaders.getFirst(Headers.CONNECTION); + if(exchange.getStatusCode() == StatusCodes.EXPECTATION_FAILED) { + //417 responses are never persistent, as we have no idea if there is a response body + //still coming on the wire. + exchange.setPersistent(false); + } if (!exchange.isPersistent()) { responseHeaders.put(Headers.CONNECTION, Headers.CLOSE.toString()); } else if (exchange.isPersistent() && connection != null) { diff --git a/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java b/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java new file mode 100644 index 0000000000..840848ff94 --- /dev/null +++ b/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java @@ -0,0 +1,83 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2020 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.undertow.server.handlers; + +import java.io.IOException; +import java.net.Socket; +import java.nio.charset.StandardCharsets; + +import org.junit.Assume; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import io.undertow.server.HttpHandler; +import io.undertow.server.HttpServerExchange; +import io.undertow.testutils.DefaultServer; +import io.undertow.util.StatusCodes; + +/** + * @author Stuart Douglas + */ +@RunWith(DefaultServer.class) +public class HttpContinueConduitWrappingHandlerBufferLeakTestCase { + + static Socket persistentSocket; + + @BeforeClass + public static void setup() { + final BlockingHandler blockingHandler = new BlockingHandler(); + final HttpContinueReadHandler handler = new HttpContinueReadHandler(blockingHandler); + DefaultServer.setRootHandler(handler); + blockingHandler.setRootHandler(new HttpHandler() { + @Override + public void handleRequest(final HttpServerExchange exchange) { + try { + exchange.getRequestChannel(); + exchange.setStatusCode(StatusCodes.EXPECTATION_FAILED); + exchange.getOutputStream().close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }); + } + + @Before + public void before() { + Assume.assumeFalse(DefaultServer.isAjp()); + } + + @Test + public void testHttpContinueRejectedBodySentAnywayNoBufferLeak() throws IOException { + persistentSocket = new Socket(DefaultServer.getHostAddress(), DefaultServer.getHostPort()); + + String message = "POST /path HTTP/1.1\r\n" + + "Expect: 100-continue\r\n" + + "Content-Length: 16\r\n" + + "Content-Type: text/plain; charset=ISO-8859-1\r\n" + + "Host: localhost:7777\r\n" + + "Connection: Keep-Alive\r\n\r\nMy HTTP Request!"; + persistentSocket.getOutputStream().write(message.getBytes(StandardCharsets.UTF_8)); + persistentSocket.getOutputStream().flush(); + persistentSocket.getInputStream().read(); + } + +} From aa5e1fe11fec75032f14f0ae23e586f4cf3a3365 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Fri, 28 Feb 2020 12:34:05 +1100 Subject: [PATCH 2/4] [UNDERTOW-1657] Fix potential buffer leak in HTTP Continue handling --- .../handlers/HttpContinueReadHandler.java | 14 ++ .../server/protocol/http/HttpContinue.java | 3 + .../protocol/http/HttpResponseConduit.java | 42 ++-- .../protocol/http/HttpServerConnection.java | 10 +- ...duitWrappingHandlerBufferLeakTestCase.java | 23 ++- .../handlers/HttpContinueServletTestCase.java | 191 ++++++++++++++++++ 6 files changed, 262 insertions(+), 21 deletions(-) create mode 100644 servlet/src/test/java/io/undertow/servlet/test/handlers/HttpContinueServletTestCase.java diff --git a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java index 2c482d7979..1d91cd233e 100644 --- a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java +++ b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java @@ -25,11 +25,14 @@ import io.undertow.server.ConduitWrapper; import io.undertow.server.Connectors; +import io.undertow.server.ResponseCommitListener; import io.undertow.server.protocol.http.HttpContinue; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; import io.undertow.util.ConduitFactory; import io.undertow.util.StatusCodes; + +import org.xnio.IoUtils; import org.xnio.channels.StreamSinkChannel; import org.xnio.conduits.AbstractStreamSourceConduit; import org.xnio.conduits.StreamSourceConduit; @@ -62,6 +65,17 @@ public HttpContinueReadHandler(final HttpHandler handler) { public void handleRequest(final HttpServerExchange exchange) throws Exception { if (HttpContinue.requiresContinueResponse(exchange)) { exchange.addRequestWrapper(WRAPPER); + exchange.addResponseCommitListener(new ResponseCommitListener() { + @Override + public void beforeCommit(HttpServerExchange exchange) { + //we are writing the response, and have not read the request then we mark this as non-persistent + if (!HttpContinue.isContinueResponseSent(exchange)) { + exchange.setPersistent(false); + //we also kill the request channel, because it is unusable now + IoUtils.safeClose(exchange.getRequestChannel()); + } + } + }); } handler.handleRequest(exchange); } diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpContinue.java b/core/src/main/java/io/undertow/server/protocol/http/HttpContinue.java index 06da87e081..67a704a1f5 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpContinue.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpContinue.java @@ -91,6 +91,9 @@ public static boolean requiresContinueResponse(HeaderMap requestHeaders) { return false; } + public static boolean isContinueResponseSent(HttpServerExchange exchange) { + return exchange.getAttachment(ALREADY_SENT) != null; + } /** * Sends a continuation using async IO, and calls back when it is complete. diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpResponseConduit.java b/core/src/main/java/io/undertow/server/protocol/http/HttpResponseConduit.java index 41a5959529..0be90bbd3a 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpResponseConduit.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpResponseConduit.java @@ -18,18 +18,16 @@ package io.undertow.server.protocol.http; -import io.undertow.UndertowMessages; -import io.undertow.server.Connectors; -import io.undertow.server.HttpServerExchange; -import io.undertow.util.HeaderMap; -import io.undertow.util.HeaderValues; -import io.undertow.util.HttpString; -import io.undertow.util.Protocols; -import io.undertow.util.StatusCodes; +import static org.xnio.Bits.allAreClear; +import static org.xnio.Bits.allAreSet; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.ClosedChannelException; +import java.nio.channels.FileChannel; + import org.xnio.Buffers; import org.xnio.IoUtils; -import io.undertow.connector.ByteBufferPool; -import io.undertow.connector.PooledByteBuffer; import org.xnio.XnioWorker; import org.xnio.channels.StreamSourceChannel; import org.xnio.conduits.AbstractStreamSinkConduit; @@ -37,13 +35,16 @@ import org.xnio.conduits.Conduits; import org.xnio.conduits.StreamSinkConduit; -import java.io.IOException; -import java.nio.ByteBuffer; -import java.nio.channels.ClosedChannelException; -import java.nio.channels.FileChannel; - -import static org.xnio.Bits.allAreClear; -import static org.xnio.Bits.allAreSet; +import io.undertow.UndertowMessages; +import io.undertow.connector.ByteBufferPool; +import io.undertow.connector.PooledByteBuffer; +import io.undertow.server.Connectors; +import io.undertow.server.HttpServerExchange; +import io.undertow.util.HeaderMap; +import io.undertow.util.HeaderValues; +import io.undertow.util.HttpString; +import io.undertow.util.Protocols; +import io.undertow.util.StatusCodes; /** * @author David M. Lloyd @@ -290,6 +291,13 @@ private void bufferDone() { } } + public void freeContinueResponse() { + if (pooledBuffer != null) { + pooledBuffer.close(); + pooledBuffer = null; + } + } + private static void writeString(ByteBuffer buffer, String string) { int length = string.length(); for (int charIndex = 0; charIndex < length; charIndex++) { diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java b/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java index 1b78c0681f..179b0b9774 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java @@ -111,7 +111,15 @@ public HttpServerExchange sendOutOfBandResponse(HttpServerExchange exchange) { @Override public StreamSinkConduit wrap(ConduitFactory factory, HttpServerExchange exchange) { - ServerFixedLengthStreamSinkConduit fixed = new ServerFixedLengthStreamSinkConduit(new HttpResponseConduit(getSinkChannel().getConduit(), getByteBufferPool(), HttpServerConnection.this, exchange), false, false); + HttpResponseConduit httpResponseConduit = new HttpResponseConduit(getSinkChannel().getConduit(), getByteBufferPool(), HttpServerConnection.this, exchange); + exchange.addExchangeCompleteListener(new ExchangeCompletionListener() { + @Override + public void exchangeEvent(HttpServerExchange exchange, NextListener nextListener) { + httpResponseConduit.freeContinueResponse(); + nextListener.proceed(); + } + }); + ServerFixedLengthStreamSinkConduit fixed = new ServerFixedLengthStreamSinkConduit(httpResponseConduit, false, false); fixed.reset(0, exchange); return fixed; } diff --git a/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java b/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java index 840848ff94..9d924b7ba9 100644 --- a/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java +++ b/core/src/test/java/io/undertow/server/handlers/HttpContinueConduitWrappingHandlerBufferLeakTestCase.java @@ -50,9 +50,11 @@ public static void setup() { @Override public void handleRequest(final HttpServerExchange exchange) { try { - exchange.getRequestChannel(); - exchange.setStatusCode(StatusCodes.EXPECTATION_FAILED); - exchange.getOutputStream().close(); + if (exchange.getQueryParameters().containsKey("reject")) { + exchange.getRequestChannel(); + exchange.setStatusCode(StatusCodes.EXPECTATION_FAILED); + exchange.getOutputStream().close(); + } } catch (IOException e) { throw new RuntimeException(e); } @@ -69,6 +71,21 @@ public void before() { public void testHttpContinueRejectedBodySentAnywayNoBufferLeak() throws IOException { persistentSocket = new Socket(DefaultServer.getHostAddress(), DefaultServer.getHostPort()); + String message = "POST /path?reject=true HTTP/1.1\r\n" + + "Expect: 100-continue\r\n" + + "Content-Length: 16\r\n" + + "Content-Type: text/plain; charset=ISO-8859-1\r\n" + + "Host: localhost:7777\r\n" + + "Connection: Keep-Alive\r\n\r\nMy HTTP Request!"; + persistentSocket.getOutputStream().write(message.getBytes(StandardCharsets.UTF_8)); + persistentSocket.getOutputStream().flush(); + persistentSocket.getInputStream().read(); + } + + @Test + public void testHttpContinueBodySentAnywayNoLeak() throws IOException { + persistentSocket = new Socket(DefaultServer.getHostAddress(), DefaultServer.getHostPort()); + String message = "POST /path HTTP/1.1\r\n" + "Expect: 100-continue\r\n" + "Content-Length: 16\r\n" + diff --git a/servlet/src/test/java/io/undertow/servlet/test/handlers/HttpContinueServletTestCase.java b/servlet/src/test/java/io/undertow/servlet/test/handlers/HttpContinueServletTestCase.java new file mode 100644 index 0000000000..fd0075fc37 --- /dev/null +++ b/servlet/src/test/java/io/undertow/servlet/test/handlers/HttpContinueServletTestCase.java @@ -0,0 +1,191 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2014 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.undertow.servlet.test.handlers; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.StringEntity; +import org.apache.http.params.BasicHttpParams; +import org.apache.http.params.HttpParams; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import io.undertow.servlet.Servlets; +import io.undertow.servlet.test.util.DeploymentUtils; +import io.undertow.testutils.DefaultServer; +import io.undertow.testutils.HttpClientUtils; +import io.undertow.testutils.TestHttpClient; +import io.undertow.util.StatusCodes; + +/** + * @author Stuart Douglas + */ +@RunWith(DefaultServer.class) +public class HttpContinueServletTestCase { + + private static volatile boolean accept = false; + + @BeforeClass + public static void setup() { + DeploymentUtils.setupServlet(Servlets.servlet(ContinueConsumeServlet.class).addMappings("/path"), + Servlets.servlet(ContinueIgnoreServlet.class).addMappings("/ignore")); + } + + @Before + public void before() { + Assume.assumeFalse(DefaultServer.isAjp()); + } + + @Test + public void testHttpContinueRejected() throws IOException { + accept = false; + String message = "My HTTP Request!"; + HttpParams httpParams = new BasicHttpParams(); + httpParams.setParameter("http.protocol.wait-for-continue", Integer.MAX_VALUE); + + TestHttpClient client = new TestHttpClient(); + client.setParams(httpParams); + try { + HttpPost post = new HttpPost(DefaultServer.getDefaultServerURL() + "/servletContext/path"); + post.addHeader("Expect", "100-continue"); + post.setEntity(new StringEntity(message)); + + HttpResponse result = client.execute(post); + Assert.assertEquals(StatusCodes.EXPECTATION_FAILED, result.getStatusLine().getStatusCode()); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testHttpContinueAccepted() throws IOException { + accept = true; + String message = "My HTTP Request!"; + HttpParams httpParams = new BasicHttpParams(); + httpParams.setParameter("http.protocol.wait-for-continue", Integer.MAX_VALUE); + + TestHttpClient client = new TestHttpClient(); + client.setParams(httpParams); + try { + HttpPost post = new HttpPost(DefaultServer.getDefaultServerURL() + "/servletContext/path"); + post.addHeader("Expect", "100-continue"); + post.setEntity(new StringEntity(message)); + + HttpResponse result = client.execute(post); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + Assert.assertEquals(message, HttpClientUtils.readResponse(result)); + } finally { + client.getConnectionManager().shutdown(); + } + } + + @Test + public void testHttpContinueIgnored() throws IOException { + accept = true; + String message = "My HTTP Request!"; + HttpParams httpParams = new BasicHttpParams(); + httpParams.setParameter("http.protocol.wait-for-continue", Integer.MAX_VALUE); + + TestHttpClient client = new TestHttpClient(); + client.setParams(httpParams); + try { + HttpPost post = new HttpPost(DefaultServer.getDefaultServerURL() + "/servletContext/ignore"); + post.addHeader("Expect", "100-continue"); + post.setEntity(new StringEntity(message)); + + HttpResponse result = client.execute(post); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + Assert.assertEquals("", HttpClientUtils.readResponse(result)); + } finally { + client.getConnectionManager().shutdown(); + } + } + //UNDERTOW-162 + @Test + public void testHttpContinueAcceptedWithChunkedRequest() throws IOException { + accept = true; + String message = "My HTTP Request!"; + HttpParams httpParams = new BasicHttpParams(); + httpParams.setParameter("http.protocol.wait-for-continue", Integer.MAX_VALUE); + + TestHttpClient client = new TestHttpClient(); + client.setParams(httpParams); + try { + HttpPost post = new HttpPost(DefaultServer.getDefaultServerURL() + "/servletContext/path"); + post.addHeader("Expect", "100-continue"); + post.setEntity(new StringEntity(message) { + @Override + public long getContentLength() { + return -1; + } + }); + + HttpResponse result = client.execute(post); + Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode()); + Assert.assertEquals(message, HttpClientUtils.readResponse(result)); + } finally { + client.getConnectionManager().shutdown(); + } + } + + public static class ContinueConsumeServlet extends HttpServlet { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + try { + if (!accept) { + resp.setStatus(StatusCodes.EXPECTATION_FAILED); + return; + } + byte[] buffer = new byte[1024]; + final ByteArrayOutputStream b = new ByteArrayOutputStream(); + int r = 0; + final OutputStream outputStream = resp.getOutputStream(); + final InputStream inputStream = req.getInputStream(); + while ((r = inputStream.read(buffer)) > 0) { + b.write(buffer, 0, r); + } + outputStream.write(b.toByteArray()); + outputStream.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } + + public static class ContinueIgnoreServlet extends HttpServlet { + + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + + } + } +} From b53d4589c586e8bbdcc89ed60f32cd7977e9a4f4 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 15 Apr 2020 15:39:02 +1000 Subject: [PATCH 3/4] [UNDERTOW-1657] Fix issue with 100-continue and h2 --- .../handlers/HttpContinueReadHandler.java | 17 ++++++++--------- .../protocol/ajp/AjpServerConnection.java | 6 +++++- .../protocol/http/HttpServerConnection.java | 6 +++++- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java index 1d91cd233e..20712e5fce 100644 --- a/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java +++ b/core/src/main/java/io/undertow/server/handlers/HttpContinueReadHandler.java @@ -23,20 +23,19 @@ import java.nio.channels.FileChannel; import java.util.concurrent.TimeUnit; +import org.xnio.channels.StreamSinkChannel; +import org.xnio.conduits.AbstractStreamSourceConduit; +import org.xnio.conduits.StreamSourceConduit; + import io.undertow.server.ConduitWrapper; import io.undertow.server.Connectors; -import io.undertow.server.ResponseCommitListener; -import io.undertow.server.protocol.http.HttpContinue; import io.undertow.server.HttpHandler; import io.undertow.server.HttpServerExchange; +import io.undertow.server.ResponseCommitListener; +import io.undertow.server.protocol.http.HttpContinue; import io.undertow.util.ConduitFactory; import io.undertow.util.StatusCodes; -import org.xnio.IoUtils; -import org.xnio.channels.StreamSinkChannel; -import org.xnio.conduits.AbstractStreamSourceConduit; -import org.xnio.conduits.StreamSourceConduit; - /** * Handler for requests that require 100-continue responses. If an attempt is made to read from the source * channel then a 100 continue response is sent. @@ -48,7 +47,7 @@ public class HttpContinueReadHandler implements HttpHandler { private static final ConduitWrapper WRAPPER = new ConduitWrapper() { @Override public StreamSourceConduit wrap(final ConduitFactory factory, final HttpServerExchange exchange) { - if(exchange.isRequestChannelAvailable() && !exchange.isResponseStarted()) { + if (exchange.isRequestChannelAvailable() && !exchange.isResponseStarted()) { return new ContinueConduit(factory.create(), exchange); } return factory.create(); @@ -72,7 +71,7 @@ public void beforeCommit(HttpServerExchange exchange) { if (!HttpContinue.isContinueResponseSent(exchange)) { exchange.setPersistent(false); //we also kill the request channel, because it is unusable now - IoUtils.safeClose(exchange.getRequestChannel()); + exchange.getConnection().terminateRequestChannel(exchange); } } }); diff --git a/core/src/main/java/io/undertow/server/protocol/ajp/AjpServerConnection.java b/core/src/main/java/io/undertow/server/protocol/ajp/AjpServerConnection.java index af09ff0a75..54d1665a60 100644 --- a/core/src/main/java/io/undertow/server/protocol/ajp/AjpServerConnection.java +++ b/core/src/main/java/io/undertow/server/protocol/ajp/AjpServerConnection.java @@ -26,6 +26,8 @@ import io.undertow.server.HttpServerExchange; import io.undertow.server.SSLSessionInfo; import io.undertow.util.DateUtils; + +import org.xnio.IoUtils; import org.xnio.OptionMap; import io.undertow.connector.ByteBufferPool; import org.xnio.StreamConnection; @@ -61,7 +63,9 @@ public boolean isContinueResponseSupported() { @Override public void terminateRequestChannel(HttpServerExchange exchange) { - //todo: terminate + if (!exchange.isPersistent()) { + IoUtils.safeClose(getChannel().getSourceChannel()); + } } @Override diff --git a/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java b/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java index 179b0b9774..f15c0f0385 100644 --- a/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java +++ b/core/src/main/java/io/undertow/server/protocol/http/HttpServerConnection.java @@ -36,6 +36,8 @@ import io.undertow.util.HttpString; import io.undertow.util.ImmediatePooledByteBuffer; import io.undertow.util.Methods; + +import org.xnio.IoUtils; import org.xnio.OptionMap; import io.undertow.connector.ByteBufferPool; import io.undertow.connector.PooledByteBuffer; @@ -143,7 +145,9 @@ public boolean isContinueResponseSupported() { @Override public void terminateRequestChannel(HttpServerExchange exchange) { - + if (!exchange.isPersistent()) { + IoUtils.safeClose(getChannel().getSourceChannel()); + } } /** From 30b029f3b523e12f76566e834d64a9b9a36547f5 Mon Sep 17 00:00:00 2001 From: Bartosz Spyrko-Smietanko Date: Fri, 17 Apr 2020 14:13:44 +0100 Subject: [PATCH 4/4] [UNDERTOW-1709][JBEAP-19266] NullPointerException when calling the AJP port with invalid request --- core/src/main/java/io/undertow/UndertowLogger.java | 2 +- core/src/main/java/io/undertow/server/HttpServerExchange.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/io/undertow/UndertowLogger.java b/core/src/main/java/io/undertow/UndertowLogger.java index d586948cc2..f8d6d3ec1e 100644 --- a/core/src/main/java/io/undertow/UndertowLogger.java +++ b/core/src/main/java/io/undertow/UndertowLogger.java @@ -393,7 +393,7 @@ void nodeConfigCreated(URI connectionURI, String balancer, String domain, String @Message(id = 5084, value = "Attempted to write %s bytes however content-length has been set to %s") IOException dataLargerThanContentLength(long totalToWrite, long responseContentLength); - @LogMessage(level = ERROR) + @LogMessage(level = DEBUG) @Message(id = 5085, value = "Connection %s for exchange %s was not closed cleanly, forcibly closing connection") void responseWasNotTerminated(ServerConnection connection, HttpServerExchange exchange); diff --git a/core/src/main/java/io/undertow/server/HttpServerExchange.java b/core/src/main/java/io/undertow/server/HttpServerExchange.java index 2186a6235d..dbc98f7d46 100644 --- a/core/src/main/java/io/undertow/server/HttpServerExchange.java +++ b/core/src/main/java/io/undertow/server/HttpServerExchange.java @@ -155,7 +155,7 @@ public final class HttpServerExchange extends AbstractAttachable { // mutable state private int state = 200; - private HttpString requestMethod; + private HttpString requestMethod = HttpString.EMPTY; private String requestScheme; /**