diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java index c694176b8f75..e5ed3e95b79f 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiResponse.java @@ -196,30 +196,45 @@ public void sendRedirect(int code, String location) throws IOException @Override public void setDateHeader(String name, long date) { + if (name == null) + return; // Spec is to do nothing + getResponse().getHeaders().putDate(name, date); } @Override public void addDateHeader(String name, long date) { + if (name == null) + return; // Spec is to do nothing + getResponse().getHeaders().addDateField(name, date); } @Override public void setHeader(String name, String value) { + if (name == null) + return; // Spec is to do nothing + getResponse().getHeaders().put(name, value); } @Override public void addHeader(String name, String value) { + if (name == null || value == null) + return; // Spec is to do nothing + getResponse().getHeaders().add(name, value); } @Override public void setIntHeader(String name, int value) { + if (name == null) + return; // Spec is to do nothing + // TODO do we need int versions? if (!isCommitted()) getResponse().getHeaders().put(name, value); @@ -228,6 +243,9 @@ public void setIntHeader(String name, int value) @Override public void addIntHeader(String name, int value) { + if (name == null) + return; // Spec is to do nothing + // TODO do we need a native version? if (!isCommitted()) getResponse().getHeaders().add(name, Integer.toString(value)); diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java index 55e504efd68b..1c79ec3f2387 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java @@ -29,163 +29,58 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; +import org.eclipse.jetty.util.component.LifeCycle; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; public class ResponseHeadersTest { - public static class SimulateUpgradeServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException - { - response.setHeader("Upgrade", "WebSocket"); - response.addHeader("Connection", "Upgrade"); - response.addHeader("Sec-WebSocket-Accept", "123456789=="); - - response.setStatus(HttpServletResponse.SC_SWITCHING_PROTOCOLS); - } - } - - public static class MultilineResponseValueServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException - { - // The bad use-case - String pathInfo = URIUtil.decodePath(req.getPathInfo()); - if (pathInfo != null && pathInfo.length() > 1 && pathInfo.startsWith("/")) - { - pathInfo = pathInfo.substring(1); - } - response.setHeader("X-example", pathInfo); - - // The correct use - response.setContentType("text/plain"); - response.setCharacterEncoding("utf-8"); - response.getWriter().println("Got request uri - " + req.getRequestURI()); - } - } - - public static class CharsetResetToJsonMimeTypeServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException - { - // We set an initial desired behavior. - response.setContentType("text/html; charset=US-ASCII"); - PrintWriter writer = response.getWriter(); - - // We reset the response, as we don't want it to be HTML anymore. - response.reset(); - - // switch to json operation - // The use of application/json is always assumed to be UTF-8 - // and should never have a `charset=` entry on the `Content-Type` response header - response.setContentType("application/json"); - writer.println("{ \"what\": \"should this be?\" }"); - } - } - - public static class CharsetChangeToJsonMimeTypeServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException - { - // We set an initial desired behavior. - response.setContentType("text/html; charset=US-ASCII"); - - // switch to json behavior. - // The use of application/json is always assumed to be UTF-8 - // and should never have a `charset=` entry on the `Content-Type` response header - response.setContentType("application/json"); + private Server server; + private LocalConnector connector; - PrintWriter writer = response.getWriter(); - writer.println("{ \"what\": \"should this be?\" }"); - } - } - - public static class CharsetChangeToJsonMimeTypeSetCharsetToNullServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException - { - // We set an initial desired behavior. - - response.setContentType("text/html; charset=us-ascii"); - PrintWriter writer = response.getWriter(); - - // switch to json behavior. - // The use of application/json is always assumed to be UTF-8 - // and should never have a `charset=` entry on the `Content-Type` response header - response.setContentType("application/json"); - // attempt to indicate that there is truly no charset meant to be used in the response header - response.setCharacterEncoding(null); - - writer.println("{ \"what\": \"should this be?\" }"); - } - } - - public static class FlushResponseServlet extends HttpServlet - { - @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException - { - PrintWriter writer = response.getWriter(); - writer.println("Hello"); - writer.flush(); - if (!response.isCommitted()) - throw new IllegalStateException(); - writer.println("World"); - writer.close(); - } - } - - private static Server server; - private static LocalConnector connector; - - @BeforeAll - public static void startServer() throws Exception + public void startServer(ServletContextHandler contextHandler) throws Exception { server = new Server(); connector = new LocalConnector(server); server.addConnector(connector); - ServletContextHandler context = new ServletContextHandler(); - context.setContextPath("/"); - server.setHandler(context); - - context.addServlet(new ServletHolder(new SimulateUpgradeServlet()), "/ws/*"); - context.addServlet(new ServletHolder(new MultilineResponseValueServlet()), "/multiline/*"); - context.addServlet(CharsetResetToJsonMimeTypeServlet.class, "/charset/json-reset/*"); - context.addServlet(CharsetChangeToJsonMimeTypeServlet.class, "/charset/json-change/*"); - context.addServlet(CharsetChangeToJsonMimeTypeSetCharsetToNullServlet.class, "/charset/json-change-null/*"); - context.addServlet(FlushResponseServlet.class, "/flush/*"); - + server.setHandler(contextHandler); server.start(); } - @AfterAll - public static void stopServer() + @AfterEach + public void stopServer() { - try - { - server.stop(); - } - catch (Exception e) - { - e.printStackTrace(System.err); - } + LifeCycle.stop(server); } @Test public void testResponseWebSocketHeaderFormat() throws Exception { + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + HttpServlet fakeWsServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + response.setHeader("Upgrade", "WebSocket"); + response.addHeader("Connection", "Upgrade"); + response.addHeader("Sec-WebSocket-Accept", "123456789=="); + + response.setStatus(HttpServletResponse.SC_SWITCHING_PROTOCOLS); + } + }; + contextHandler.addServlet(fakeWsServlet, "/ws/*"); + + startServer(contextHandler); + HttpTester.Request request = new HttpTester.Request(); request.setMethod("GET"); request.setURI("/ws/"); @@ -204,6 +99,32 @@ public void testResponseWebSocketHeaderFormat() throws Exception @Test public void testMultilineResponseHeaderValue() throws Exception { + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + + HttpServlet multilineServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException + { + // The bad use-case + String pathInfo = URIUtil.decodePath(req.getPathInfo()); + if (pathInfo != null && pathInfo.length() > 1 && pathInfo.startsWith("/")) + { + pathInfo = pathInfo.substring(1); + } + response.setHeader("X-example", pathInfo); + + // The correct use + response.setContentType("text/plain"); + response.setCharacterEncoding("utf-8"); + response.getWriter().println("Got request uri - " + req.getRequestURI()); + } + }; + + contextHandler.addServlet(multilineServlet, "/multiline/*"); + startServer(contextHandler); + String actualPathInfo = "%0A%20Content-Type%3A%20image/png%0A%20Content-Length%3A%208%0A%20%0A%20yuck