From 5485450bed584b2e4ac03b1d768dfe069aa3fc37 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 12 Dec 2024 17:33:29 +1100 Subject: [PATCH] Close HttpOutput after redirect Signed-off-by: Lachlan Roberts --- .../ee10/servlet/ServletApiResponse.java | 10 ++ .../ee10/servlet/WriteAfterRedirectTest.java | 120 ++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/WriteAfterRedirectTest.java 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 45bf9b1252c4..5037a57493a5 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 @@ -186,9 +186,19 @@ public void sendRedirect(int code, String location) throws IOException { Response.sendRedirect(getServletRequestInfo().getRequest(), getResponse(), callback, code, location, false); callback.block(); + closeOutput(); } } + public void closeOutput() throws IOException + { + ServletResponseInfo info = getServletResponseInfo(); + if (info.getOutputType() == ServletContextResponse.OutputType.WRITER) + info.getWriter().close(); + else + _servletChannel.getHttpOutput().close(); + } + @Override public void setDateHeader(String name, long date) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/WriteAfterRedirectTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/WriteAfterRedirectTest.java new file mode 100644 index 000000000000..fe205ce9e450 --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/WriteAfterRedirectTest.java @@ -0,0 +1,120 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.ee10.servlet; + +import java.io.IOException; +import java.io.OutputStream; +import java.io.PrintWriter; +import java.net.URI; +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.URIUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; + +public class WriteAfterRedirectTest +{ + private Server _server; + private URI _uri; + private HttpClient _client; + + public void startServer(HttpServlet servlet) throws Exception + { + _server = new Server(); + ServerConnector connector = new ServerConnector(_server); + _server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + context.addServlet(servlet, "/*"); + _server.setHandler(context); + _server.start(); + _uri = URI.create("http://localhost:" + connector.getLocalPort() + "/"); + + _client = new HttpClient(); + _client.start(); + } + + @AfterEach + public void stopServer() throws Exception + { + _client.stop(); + _server.stop(); + } + + @Test + public void testWriteAfterRedirect() throws Exception + { + AtomicReference errorReference = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + String pathInContext = URIUtil.addPaths(req.getServletPath(), req.getPathInfo()); + if (pathInContext.startsWith("/redirect")) + { + OutputStream out = resp.getOutputStream(); + resp.sendRedirect("/hello"); + try + { + out.write('x'); + } + catch (Throwable t) + { + errorReference.set(t); + throw t; + } + } + else + { + PrintWriter writer = resp.getWriter(); + writer.print("hello world"); + } + } + }); + + // We get the correct redirect. + _client.setFollowRedirects(false); + ContentResponse response = _client.GET(_uri.resolve("redirect")); + assertThat(response.getStatus(), is(HttpServletResponse.SC_MOVED_TEMPORARILY)); + assertThat(response.getHeaders().get(HttpHeader.CONTENT_LENGTH), is("0")); + assertThat(response.getContent().length, is(0)); + assertThat(response.getHeaders().get(HttpHeader.LOCATION), is("/hello")); + + // Following the redirect gives the hello page. + _client.setFollowRedirects(true); + response = _client.GET(_uri.resolve("redirect")); + assertThat(response.getStatus(), is(HttpServletResponse.SC_OK)); + assertThat(response.getContentAsString(), equalTo("hello world")); + + // The write() in the servlet actually threw because the HttpOutput was closed. + assertThat(errorReference.get(), instanceOf(IOException.class)); + assertThat(errorReference.get().getMessage(), containsString("Closed")); + } +}