From d3120ee2122f4ed6984e0193552049b7a93810f0 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Mar 2018 14:29:10 -0500 Subject: [PATCH 1/4] Issue #2307 - On sendError() the response character encoding is null + Added testcase to replicate + Making character encoding from unset during sendError Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/Response.java | 1 + .../jetty/server/ErrorHandlerTest.java | 62 +++++++++++++++---- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 9f7f00aff17..c35dae1f609 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -621,6 +621,7 @@ public class Response implements HttpServletResponse _mimeType=null; _characterEncoding=null; + _encodingFrom = EncodingFrom.NOT_SET; _outputType = OutputType.NONE; setHeader(HttpHeader.EXPIRES,null); setHeader(HttpHeader.LAST_MODIFIED,null); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java index eff49a97385..64a3f9b8087 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java @@ -19,30 +19,34 @@ package org.eclipse.jetty.server; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; -import static org.junit.Assert.*; +import static org.junit.Assert.assertThat; import java.io.IOException; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ErrorHandler; -import org.eclipse.jetty.toolchain.test.AdvancedRunner; -import org.junit.After; -import org.junit.Before; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Test; -import org.junit.runner.RunWith; -@RunWith(AdvancedRunner.class) public class ErrorHandlerTest { - Server server; - LocalConnector connector; + static Server server; + static LocalConnector connector; - @Before - public void before() throws Exception + @BeforeClass + public static void before() throws Exception { server = new Server(); connector = new LocalConnector(server); @@ -72,17 +76,36 @@ public class ErrorHandlerTest .append("}"); break; } + case "text/plain": + { + baseRequest.setHandled(true); + response.setContentType("text/plain"); + response.getOutputStream().print(response.getContentType()); + break; + } default: super.generateAcceptableResponse(baseRequest,request,response,code,message,mimeType); } } }); + server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + if(target.startsWith("/charencoding/")) + { + response.setCharacterEncoding("utf-8"); + response.sendError(404); + } + } + }); server.start(); } - @After - public void after() throws Exception + @AfterClass + public static void after() throws Exception { server.stop(); } @@ -260,4 +283,19 @@ public class ErrorHandlerTest assertThat(response,containsString("Content-Type: text/json")); } + @Test + public void testCharEncoding() throws Exception + { + String rawResponse = connector.getResponse( + "GET /charencoding/foo HTTP/1.1\r\n"+ + "Host: Localhost\r\n"+ + "Accept: text/plain\r\n"+ + "\r\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + + assertThat("Response status code", response.getStatus(), is(404)); + HttpField contentType = response.getField(HttpHeader.CONTENT_TYPE); + assertThat("Response Content-Type", contentType, is(notNullValue())); + assertThat("Response Content-Type value", contentType.getValue(), not(containsString("null"))); + } } From dcfa6f0f380f5545c4e48ae3f70bae4e0ee6b0a3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Mar 2018 17:33:58 -0500 Subject: [PATCH 2/4] Issue #2307 - applying changes from review Signed-off-by: Joakim Erdfelt --- .../src/main/java/org/eclipse/jetty/server/Response.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index c35dae1f609..301081e2d70 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -619,10 +619,9 @@ public class Response implements HttpServletResponse } - _mimeType=null; - _characterEncoding=null; - _encodingFrom = EncodingFrom.NOT_SET; _outputType = OutputType.NONE; + setContentType(null); + setCharacterEncoding(null); setHeader(HttpHeader.EXPIRES,null); setHeader(HttpHeader.LAST_MODIFIED,null); setHeader(HttpHeader.CACHE_CONTROL,null); From a810ecf67e272d17b55bdb9500b2fb8f54cbf7bf Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 14 Mar 2018 15:59:37 +1100 Subject: [PATCH 3/4] Fixed test with atomic the --task was not protected with a memory barrier, so different producing threads could contend on the field. Signed-off-by: Greg Wilkins --- .../jetty/util/thread/strategy/ExecutionStrategyTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/strategy/ExecutionStrategyTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/strategy/ExecutionStrategyTest.java index 1e4cb87cd19..93b0a21779e 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/strategy/ExecutionStrategyTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/strategy/ExecutionStrategyTest.java @@ -156,11 +156,11 @@ public class ExecutionStrategyTest Producer producer = new TestProducer() { - int tasks=TASKS; + AtomicInteger tasks = new AtomicInteger(TASKS); @Override public Runnable produce() { - final int id = --tasks; + final int id = tasks.decrementAndGet(); if (id>=0) { while(_threads.isRunning()) From ed907f8ed9ff1d87406e2343e600e8f9775ad7f3 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 14 Mar 2018 16:05:16 +1100 Subject: [PATCH 4/4] more generous test times Signed-off-by: Greg Wilkins --- .../eclipse/jetty/server/LowResourcesMonitorTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java index ddf14534eff..121ef30d5cf 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java @@ -218,28 +218,28 @@ public class LowResourcesMonitorTest @Test public void testMaxLowResourceTime() throws Exception { - _lowResourcesMonitor.setMaxLowResourcesTime(2000); + _lowResourcesMonitor.setMaxLowResourcesTime(5000); Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); try(Socket socket0 = new Socket("localhost",_connector.getLocalPort())) { _lowResourcesMonitor.setMaxMemory(1); - Thread.sleep(1200); + Thread.sleep(2400); Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); try(Socket socket1 = new Socket("localhost",_connector.getLocalPort())) { - Thread.sleep(1200); + Thread.sleep(2400); Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); Assert.assertEquals(-1,socket0.getInputStream().read()); socket1.getOutputStream().write("G".getBytes(StandardCharsets.UTF_8)); - Thread.sleep(1200); + Thread.sleep(2400); Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); socket1.getOutputStream().write("E".getBytes(StandardCharsets.UTF_8)); - Thread.sleep(1200); + Thread.sleep(2400); Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); Assert.assertEquals(-1,socket1.getInputStream().read()); }