Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Issue #4176 setHeader after sendError (#4181) 

* Issue #4176 setHeader after sendError

SendError now makes the response immutable for headers and status.

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4176 setHeader after sendError

cleanup after review
better names

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4176 setHeader after sendError

better name

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Issue #4176 setHeader after sendError

even better name

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-10-10 19:35:28 +11:00 committed by GitHub
parent 53ed8f346c
commit ce41c122a6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 80 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View File

@ -373,7 +373,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
if (!_request.hasMetaData())
throw new IllegalStateException("state=" + _state);
_request.setHandled(false);
_response.getHttpOutput().reopen();
_response.reopen();
dispatch(DispatcherType.REQUEST, () ->
{
@ -392,7 +392,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
case ASYNC_DISPATCH:
{
_request.setHandled(false);
_response.getHttpOutput().reopen();
_response.reopen();
dispatch(DispatcherType.ASYNC,() -> getServer().handleAsync(this));
break;
@ -409,7 +409,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
// Get ready to send an error response
_request.setHandled(false);
_response.resetContent();
_response.getHttpOutput().reopen();
_response.reopen();
// the following is needed as you cannot trust the response code and reason
// as those could have been modified after calling sendError

2
jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
View File

@ -900,8 +900,8 @@ public class HttpChannelState
if (_outputState != OutputState.OPEN)
throw new IllegalStateException("Response is " + _outputState);
response.getHttpOutput().closedBySendError();
response.setStatus(code);
response.closedBySendError();
request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext());
request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());

111
jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
View File

@ -18,7 +18,6 @@
package org.eclipse.jetty.server;
import java.io.Closeable;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.channels.IllegalSelectorException;
@ -28,7 +27,6 @@ import java.util.EnumSet;
import java.util.Iterator;
import java.util.ListIterator;
import java.util.Locale;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;
import javax.servlet.ServletOutputStream;
import javax.servlet.ServletResponse;
@ -58,6 +56,7 @@ import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.server.session.SessionHandler;
import org.eclipse.jetty.util.AtomicBiInteger;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
@ -87,7 +86,7 @@ public class Response implements HttpServletResponse
private final HttpChannel _channel;
private final HttpFields _fields = new HttpFields();
private final AtomicInteger _include = new AtomicInteger();
private final AtomicBiInteger _errorSentAndIncludes = new AtomicBiInteger(); // hi is errorSent flag, lo is include count
private final HttpOutput _out;
private int _status = HttpStatus.OK_200;
private String _reason;
@ -140,19 +139,44 @@ public class Response implements HttpServletResponse
return _out;
}
public void reopen()
{
setErrorSent(false);
_out.reopen();
}
public void closedBySendError()
{
setErrorSent(true);
_out.closedBySendError();
}
/**
* @return true if the response is mutable, ie not errorSent and not included
*/
private boolean isMutable()
{
return _errorSentAndIncludes.get() == 0;
}
private void setErrorSent(boolean errorSent)
{
_errorSentAndIncludes.getAndSetHi(errorSent ? 1 : 0);
}
public boolean isIncluding()
{
return _include.get() > 0;
return _errorSentAndIncludes.getLo() > 0;
}
public void include()
{
_include.incrementAndGet();
_errorSentAndIncludes.add(0, 1);
}
public void included()
{
_include.decrementAndGet();
_errorSentAndIncludes.add(0, -1);
if (_outputType == OutputType.WRITER)
{
_writer.reopen();
@ -438,7 +462,7 @@ public class Response implements HttpServletResponse
if ((code < HttpServletResponse.SC_MULTIPLE_CHOICES) || (code >= HttpServletResponse.SC_BAD_REQUEST))
throw new IllegalArgumentException("Not a 3xx redirect code");
if (isIncluding())
if (!isMutable())
return;
if (location == null)
@ -484,34 +508,34 @@ public class Response implements HttpServletResponse
@Override
public void setDateHeader(String name, long date)
{
if (!isIncluding())
if (isMutable())
_fields.putDateField(name, date);
}
@Override
public void addDateHeader(String name, long date)
{
if (!isIncluding())
if (isMutable())
_fields.addDateField(name, date);
}
public void setHeader(HttpHeader name, String value)
{
if (HttpHeader.CONTENT_TYPE == name)
setContentType(value);
else
if (isMutable())
{
if (isIncluding())
return;
_fields.put(name, value);
if (HttpHeader.CONTENT_LENGTH == name)
if (HttpHeader.CONTENT_TYPE == name)
setContentType(value);
else
{
if (value == null)
_contentLength = -1L;
else
_contentLength = Long.parseLong(value);
_fields.put(name, value);
if (HttpHeader.CONTENT_LENGTH == name)
{
if (value == null)
_contentLength = -1L;
else
_contentLength = Long.parseLong(value);
}
}
}
}
@ -519,17 +543,21 @@ public class Response implements HttpServletResponse
@Override
public void setHeader(String name, String value)
{
long biInt = _errorSentAndIncludes.get();
if (biInt != 0)
{
boolean errorSent = AtomicBiInteger.getHi(biInt) != 0;
boolean including = AtomicBiInteger.getLo(biInt) > 0;
if (!errorSent && including && name.startsWith(SET_INCLUDE_HEADER_PREFIX))
name = name.substring(SET_INCLUDE_HEADER_PREFIX.length());
else
return;
}
if (HttpHeader.CONTENT_TYPE.is(name))
setContentType(value);
else
{
if (isIncluding())
{
if (name.startsWith(SET_INCLUDE_HEADER_PREFIX))
name = name.substring(SET_INCLUDE_HEADER_PREFIX.length());
else
return;
}
_fields.put(name, value);
if (HttpHeader.CONTENT_LENGTH.is(name))
{
@ -565,9 +593,12 @@ public class Response implements HttpServletResponse
@Override
public void addHeader(String name, String value)
{
if (isIncluding())
long biInt = _errorSentAndIncludes.get();
if (biInt != 0)
{
if (name.startsWith(SET_INCLUDE_HEADER_PREFIX))
boolean errorSent = AtomicBiInteger.getHi(biInt) != 0;
boolean including = AtomicBiInteger.getLo(biInt) > 0;
if (!errorSent && including && name.startsWith(SET_INCLUDE_HEADER_PREFIX))
name = name.substring(SET_INCLUDE_HEADER_PREFIX.length());
else
return;
@ -591,7 +622,7 @@ public class Response implements HttpServletResponse
@Override
public void setIntHeader(String name, int value)
{
if (!isIncluding())
if (isMutable())
{
_fields.putLongField(name, value);
if (HttpHeader.CONTENT_LENGTH.is(name))
@ -602,7 +633,7 @@ public class Response implements HttpServletResponse
@Override
public void addIntHeader(String name, int value)
{
if (!isIncluding())
if (isMutable())
{
_fields.add(name, Integer.toString(value));
if (HttpHeader.CONTENT_LENGTH.is(name))
@ -615,7 +646,7 @@ public class Response implements HttpServletResponse
{
if (sc <= 0)
throw new IllegalArgumentException();
if (!isIncluding())
if (isMutable())
{
// Null the reason only if the status is different. This allows
// a specific reason to be sent with setStatusWithReason followed by sendError.
@ -636,7 +667,7 @@ public class Response implements HttpServletResponse
{
if (sc <= 0)
throw new IllegalArgumentException();
if (!isIncluding())
if (isMutable())
{
_status = sc;
_reason = sm;
@ -742,7 +773,7 @@ public class Response implements HttpServletResponse
// Protect from setting after committed as default handling
// of a servlet HEAD request ALWAYS sets _content length, even
// if the getHandling committed the response!
if (isCommitted() || isIncluding())
if (isCommitted() || !isMutable())
return;
if (len > 0)
@ -818,7 +849,7 @@ public class Response implements HttpServletResponse
// Protect from setting after committed as default handling
// of a servlet HEAD request ALWAYS sets _content length, even
// if the getHandling committed the response!
if (isCommitted() || isIncluding())
if (isCommitted() || !isMutable())
return;
_contentLength = len;
_fields.putLongField(HttpHeader.CONTENT_LENGTH.toString(), len);
@ -838,7 +869,7 @@ public class Response implements HttpServletResponse
private void setCharacterEncoding(String encoding, EncodingFrom from)
{
if (isIncluding() || isWriting())
if (!isMutable() || isWriting())
return;
if (_outputType != OutputType.WRITER && !isCommitted())
@ -891,7 +922,7 @@ public class Response implements HttpServletResponse
@Override
public void setContentType(String contentType)
{
if (isCommitted() || isIncluding())
if (isCommitted() || !isMutable())
return;
if (contentType == null)
@ -1146,7 +1177,7 @@ public class Response implements HttpServletResponse
@Override
public void setLocale(Locale locale)
{
if (locale == null || isCommitted() || isIncluding())
if (locale == null || isCommitted() || !isMutable())
return;
_locale = locale;

26
jetty-server/src/test/java/org/eclipse/jetty/server/ErrorHandlerTest.java
View File

@ -53,32 +53,6 @@ public class ErrorHandlerTest
connector = new LocalConnector(server);
server.addConnector(connector);
server.setHandler(new AbstractHandler()
{
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
if (baseRequest.getDispatcherType() == DispatcherType.ERROR)
{
baseRequest.setHandled(true);
response.sendError(((Integer)request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE)).intValue());
return;
}
if (target.startsWith("/charencoding/"))
{
baseRequest.setHandled(true);
response.setCharacterEncoding("utf-8");
response.sendError(404);
return;
}
if (target.startsWith("/badmessage/"))
{
throw new ServletException(new BadMessageException(Integer.valueOf(target.substring(12))));
}
}
});
server.setHandler(new AbstractHandler()
{
@Override

5
jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java
View File

@ -689,9 +689,14 @@ public class ResponseTest
else
response.sendError(code, message);
assertTrue(response.getHttpOutput().isClosed());
assertEquals(code, response.getStatus());
assertEquals(null, response.getReason());
response.setHeader("Should-Be-Ignored", "value");
assertFalse(response.getHttpFields().containsKey("Should-Be-Ignored"));
assertEquals(expectedMessage, response.getHttpChannel().getRequest().getAttribute(RequestDispatcher.ERROR_MESSAGE));
assertThat(response.getHttpChannel().getState().unhandle(), is(HttpChannelState.Action.SEND_ERROR));
assertThat(response.getHttpChannel().getState().unhandle(), is(HttpChannelState.Action.COMPLETE));