Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sendError(-1) is an abort #12206

Merged
merged 1 commit into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.eclipse.jetty.http.MultiPartConfig;
import org.eclipse.jetty.http.Trailers;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.internal.CompletionStreamWrapper;
import org.eclipse.jetty.server.internal.HttpChannelState;
import org.eclipse.jetty.util.Attributes;
Expand Down Expand Up @@ -716,6 +717,7 @@ interface Handler extends Invocable
* is returned, then this method must not generate a response, nor complete the callback.
* @throws Exception if there is a failure during the handling. Catchers cannot assume that the callback will be
* called and thus should attempt to complete the request as if a false had been returned.
* @see AbortException
*/
boolean handle(Request request, Response response, Callback callback) throws Exception;

Expand All @@ -725,6 +727,34 @@ default InvocationType getInvocationType()
{
return InvocationType.BLOCKING;
}

/**
* A marker {@link Exception} that can be passed the {@link Callback#failed(Throwable)} of the {@link Callback}
* passed in {@link #handle(Request, Response, Callback)}, to cause request handling to be aborted. For HTTP/1
* an abort is handled with a {@link EndPoint#close()}, for later versions of HTTP, a reset message will be sent.
*/
class AbortException extends Exception
{
public AbortException()
{
super();
}

public AbortException(String message)
{
super(message);
}

public AbortException(String message, Throwable cause)
{
super(message, cause);
}

public AbortException(Throwable cause)
{
super(cause);
}
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1590,18 +1590,18 @@ public void failed(Throwable failure)

httpChannelState._callbackFailure = failure;

// Consume any input.
Throwable unconsumed = stream.consumeAvailable();
ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed);
if (!stream.isCommitted() && !(failure instanceof Request.Handler.AbortException))
{
// Consume any input.
Throwable unconsumed = stream.consumeAvailable();
ExceptionUtil.addSuppressedIfNotAssociated(failure, unconsumed);

ChannelResponse response = httpChannelState._response;
if (LOG.isDebugEnabled())
LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", stream.isCommitted(), response.isCommitted(), this);
ChannelResponse response = httpChannelState._response;
if (LOG.isDebugEnabled())
LOG.debug("failed stream.isCommitted={}, response.isCommitted={} {}", stream.isCommitted(), response.isCommitted(), this);

// There may have been an attempt to write an error response that failed.
// Do not try to write again an error response if already committed.
if (!stream.isCommitted())
errorResponse = new ErrorResponse(request);
}
}

if (errorResponse != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void sendError(int sc, String msg) throws IOException
{
switch (sc)
{
case -1 -> getServletChannel().abort(new IOException(msg));
case -1 -> getServletChannel().abort(new Request.Handler.AbortException(msg));
case HttpStatus.PROCESSING_102, HttpStatus.EARLY_HINTS_103 ->
{
if (!isCommitted())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@

package org.eclipse.jetty.ee10.servlet;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.PrintWriter;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -50,6 +54,7 @@
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.Callback;
Expand All @@ -70,6 +75,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -780,6 +786,220 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw
assertThat(responseBody, Matchers.containsString("ERROR_REQUEST_URI: /fail/599"));
}

@Test
public void testAbortWithSendError() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
contextHandler.setContextPath("/");

HttpServlet failServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
response.sendError(-1);
}
};

contextHandler.addServlet(failServlet, "/abort");
startServer(contextHandler);

ServerConnector connector = new ServerConnector(_server);
connector.setPort(0);
_server.addConnector(connector);
connector.start();
try (Socket socket = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = socket.getOutputStream();

String request = """
GET /abort HTTP/1.1\r
Host: test\r
\r
""";
output.write(request.getBytes(StandardCharsets.UTF_8));
output.flush();

BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
String line = in.readLine();
assertNull(line);
}
}

@Test
public void testAbortWithSendErrorChunked() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
contextHandler.setContextPath("/");

HttpServlet failServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
response.getOutputStream().write("test".getBytes(StandardCharsets.UTF_8));
response.flushBuffer();
response.sendError(-1);
}
};

contextHandler.addServlet(failServlet, "/abort");
startServer(contextHandler);

ServerConnector connector = new ServerConnector(_server);
connector.setPort(0);
_server.addConnector(connector);
connector.start();
try (Socket socket = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = socket.getOutputStream();

String request = """
GET /abort HTTP/1.1\r
Host: test\r
\r
""";
output.write(request.getBytes(StandardCharsets.UTF_8));
output.flush();

BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
String line = in.readLine();
assertThat(line, is("HTTP/1.1 200 OK"));

boolean chunked = false;
while (!line.isEmpty())
{
line = in.readLine();
assertNotNull(line);
chunked |= line.equals("Transfer-Encoding: chunked");
}
assertTrue(chunked);

line = in.readLine();
assertThat(line, is("4"));
line = in.readLine();
assertThat(line, is("test"));

line = in.readLine();
assertNull(line);
}
}

@Test
public void testAbortWithSendErrorContent() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
contextHandler.setContextPath("/");

HttpServlet failServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
response.setContentLength(10);
response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8));
response.flushBuffer();
response.sendError(-1);
}
};

contextHandler.addServlet(failServlet, "/abort");
startServer(contextHandler);

ServerConnector connector = new ServerConnector(_server);
connector.setPort(0);
_server.addConnector(connector);
connector.start();
try (Socket socket = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = socket.getOutputStream();

String request = """
GET /abort HTTP/1.1\r
Host: test\r
\r
""";
output.write(request.getBytes(StandardCharsets.UTF_8));
output.flush();

BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
String line = in.readLine();
assertThat(line, is("HTTP/1.1 200 OK"));

boolean chunked = false;
while (!line.isEmpty())
{
line = in.readLine();
assertNotNull(line);
chunked |= line.equals("Transfer-Encoding: chunked");
}
assertFalse(chunked);

line = in.readLine();
assertThat(line, is("test"));

line = in.readLine();
assertNull(line);
}
}

@Test
public void testAbortWithSendErrorComplete() throws Exception
{
ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.NO_SECURITY | ServletContextHandler.NO_SESSIONS);
contextHandler.setContextPath("/");

HttpServlet failServlet = new HttpServlet()
{
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse response) throws ServletException, IOException
{
response.setContentLength(6);
response.getOutputStream().write("test\r\n".getBytes(StandardCharsets.UTF_8));
response.sendError(-1);
}
};

contextHandler.addServlet(failServlet, "/abort");
startServer(contextHandler);

ServerConnector connector = new ServerConnector(_server);
connector.setPort(0);
_server.addConnector(connector);
connector.start();
try (Socket socket = new Socket("localhost", connector.getLocalPort()))
{
OutputStream output = socket.getOutputStream();

String request = """
GET /abort HTTP/1.1\r
Host: test\r
\r
""";
output.write(request.getBytes(StandardCharsets.UTF_8));
output.flush();

BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
String line = in.readLine();
assertThat(line, is("HTTP/1.1 200 OK"));

boolean chunked = false;
while (!line.isEmpty())
{
line = in.readLine();
assertNotNull(line);
chunked |= line.equals("Transfer-Encoding: chunked");
}
assertFalse(chunked);

line = in.readLine();
assertThat(line, is("test"));

line = in.readLine();
assertNull(line);
}
}

@Test
public void testErrorCodeNoDefaultServletNonExistentErrorLocation() throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ public void sendError(int code, String message) throws IOException

switch (code)
{
case -1 -> _channel.abort(new IOException(message));
case -1 -> _channel.abort(new org.eclipse.jetty.server.Request.Handler.AbortException(message));
case HttpStatus.PROCESSING_102 -> sendProcessing();
case HttpStatus.EARLY_HINTS_103 -> sendEarlyHint();
default -> _channel.getState().sendError(code, message);
Expand Down
Loading
Loading