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

fixes for Response.writeError with servlet error dispatch #12698

Open
wants to merge 5 commits into
base: jetty-12.1.x
Choose a base branch
from
Open
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 @@ -23,6 +23,7 @@
import org.eclipse.jetty.security.internal.DeferredAuthenticationState;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.Callback;

/**
Expand Down Expand Up @@ -179,6 +180,19 @@ static boolean logout(Request request, Response response)
return false;
}

static AuthenticationState writeError(Request request, Response response, Callback callback, int code)
{
if (request.getContext().getErrorHandler() instanceof ErrorHandler errorHandler)
{
return errorHandler.writeError(request, response, callback, HttpStatus.FORBIDDEN_403)
? AuthenticationState.SEND_FAILURE
: new AuthenticationState.ServeAs(request.getHttpURI());
}

Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403);
return AuthenticationState.SEND_FAILURE;
}

/**
* A successful Authentication with User information.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ public AuthenticationState validateRequest(Request req, Response res, Callback c
if (charset != null)
value += ", charset=\"" + charset.name() + "\"";
res.getHeaders().put(HttpHeader.WWW_AUTHENTICATE.asString(), value);

// Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch.
Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401);
return AuthenticationState.CHALLENGE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ else if (n == 0)
"\", algorithm=MD5" +
", qop=\"auth\"" +
", stale=" + stale);
Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401);

// Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch.
Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401);
return AuthenticationState.CHALLENGE;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ private AuthenticationState dispatch(String path, Request request, Response resp
private AuthenticationState sendError(Request request, Response response, Callback callback)
{
if (_formErrorPage == null)
Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403);
return AuthenticationState.writeError(request, response, callback, HttpStatus.FORBIDDEN_403);
else if (_dispatch)
return dispatch(_formErrorPage, request, response, callback);
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ else if (httpSession != null)
private void sendChallenge(Request req, Response res, Callback callback, String token)
{
setSpnegoToken(res, token);
// Don't use AuthenticationState.writeError, to avoid possibility of doing a Servlet error dispatch.
Response.writeError(req, res, callback, HttpStatus.UNAUTHORIZED_401);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,8 @@ public String getAuthenticationType()
public AuthenticationState validateRequest(Request req, Response res, Callback callback) throws ServerAuthException
{
if (!(req.getAttribute(EndPoint.SslSessionData.ATTRIBUTE) instanceof EndPoint.SslSessionData sslSessionData))
{
Response.writeError(req, res, callback, HttpStatus.FORBIDDEN_403);
return AuthenticationState.SEND_FAILURE;
}

return AuthenticationState.writeError(req, res, callback, HttpStatus.FORBIDDEN_403);

X509Certificate[] certs = sslSessionData.peerCertificates();

try
Expand Down Expand Up @@ -106,10 +103,7 @@ public AuthenticationState validateRequest(Request req, Response res, Callback c
}

if (!AuthenticationState.Deferred.isDeferred(res))
{
Response.writeError(req, res, callback, HttpStatus.FORBIDDEN_403);
return AuthenticationState.SEND_FAILURE;
}
return AuthenticationState.writeError(req, res, callback, HttpStatus.FORBIDDEN_403);

return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,25 @@ public boolean errorPageForMethod(String method)
return ERROR_METHODS.contains(method);
}

/**
* Write an error response.
* <p>
* In Servlet implementations of the {@link ErrorHandler}, this method is overridden to signal that
* a sendError should be triggered a when it enters the servlet channel.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* a sendError should be triggered a when it enters the servlet channel.
* a sendError should be triggered a when it enters the servlet channel.
* The default implementation calls {@link Response#writeError(...)}.

The link is not correct, but you get the idea

* <p>
*
* @param request The request.
* @param response The response.
* @param callback The callback to call when the response is written.
* @param code The error status code.
* @return True if the error response was written.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return True if the error response was written.
* @return {@code True} if the error response was written; {@code False} if the state of the request and/or response has been changed so that by continuing normal handling an error response will be generated by a down stream handler.

*/
public boolean writeError(Request request, Response response, Callback callback, int code)
{
Response.writeError(request, response, callback, code);
return true;
}

@Override
public boolean handle(Request request, Response response, Callback callback) throws Exception
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,7 @@ public AuthenticationState validateRequest(JaspiMessageInfo messageInfo) throws
}
if (authStatus == AuthStatus.FAILURE)
{
Response.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN);
return AuthenticationState.SEND_FAILURE;
return AuthenticationState.writeError(messageInfo.getBaseRequest(), messageInfo.getBaseResponse(), messageInfo.getCallback(), HttpServletResponse.SC_FORBIDDEN);
}
// should not happen
throw new IllegalStateException("No AuthStatus returned");
Expand Down
Loading
Loading