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

Issue #5605 unconsumed input on sendError (#5637) 

* Issue #5605 unconsumed input on sendError

Add Connection:close if content can't be consumed during a sendError. Processed after the request has returned to the container.

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

* Update from review

 + Add close on all uncommitted requests when content cannot be consumed.

* Update from review

 + fixed comment
 + space comma

* Only consume input in COMPLETE if response is >=200 (ie not an upgrade or similar)

* Updated to be less adventurous

I do not think it was valid to always consumeAll in COMPLETE as this could break upgrades with both 101s and 200s
Instead I have reverted to having this consumeAll logic only:
 + in sendError once control has passed back to the container and we are about to generate an error page.
 + in front of all the sendRedirection that we do without calling the application first.

Extra tests also added

* Updated to be less adventurous

reverted test

* Testcase for odd sendError(400) issue.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Fix for odd sendError(400) issue.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

* Testcase for odd sendError(400) issue.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>

* Always try to consumeAll on all requests

* Refinements after testing in 10

* Refinements after testing in 10

Fixed test

* Fixed comment from review

* Updates from review

+ added redirect methods that consumeAll
+ ensureContentConsumedOrConnectionClose renamed to ensureConsumeAllOrNotPersistent
+ ensureConsumeAllOrNotPersistent now handles HTTP/1.0 and HTTP/1.1 differently

* better consumeAll implementation

* update from review

 + better javadoc
 + filter out keep-alive
 + added more tests

* update from review

 + better javadoc

* update from review

 + fixed form redirection test for http 1.0 and 1.1

* update from review

 + HttpGenerator removes keep-alive if close present
 + Use isRedirection

Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
This commit is contained in:
Greg Wilkins 2020-11-18 10:40:05 +01:00 committed by GitHub
parent 1448444c65
commit 14f94f738d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 663 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
View File

@ -23,6 +23,8 @@ import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jetty.http.HttpTokens.EndOfContent;
import org.eclipse.jetty.util.ArrayTrie;
@ -658,17 +660,23 @@ public class HttpGenerator
case CONNECTION:
{
putTo(field, header);
boolean keepAlive = field.contains(HttpHeaderValue.KEEP_ALIVE.asString());
if (keepAlive && info.getHttpVersion() == HttpVersion.HTTP_1_0 && _persistent == null)
{
_persistent = true;
}
if (field.contains(HttpHeaderValue.CLOSE.asString()))
{
close = true;
_persistent = false;
}
if (info.getHttpVersion() == HttpVersion.HTTP_1_0 && _persistent == null && field.contains(HttpHeaderValue.KEEP_ALIVE.asString()))
if (keepAlive && _persistent == Boolean.FALSE)
{
_persistent = true;
field = new HttpField(HttpHeader.CONNECTION,
Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s))
.collect(Collectors.joining(", ")));
}
putTo(field, header);
break;
}

16
jetty-http/src/test/java/org/eclipse/jetty/http/HttpGeneratorServerTest.java
View File

@ -862,4 +862,20 @@ public class HttpGeneratorServerTest
assertThat(headers, containsString(HttpHeaderValue.KEEP_ALIVE.asString()));
assertThat(headers, containsString(customValue));
}
@Test
public void testKeepAliveWithClose() throws Exception
{
HttpGenerator generator = new HttpGenerator();
HttpFields fields = new HttpFields();
fields.put(HttpHeader.CONNECTION,
HttpHeaderValue.KEEP_ALIVE.asString() + ", other, " + HttpHeaderValue.CLOSE.asString());
MetaData.Response info = new MetaData.Response(HttpVersion.HTTP_1_0, 200, "OK", fields, -1);
ByteBuffer header = BufferUtil.allocate(4096);
HttpGenerator.Result result = generator.generateResponse(info, false, header, null, null, true);
assertSame(HttpGenerator.Result.FLUSH, result);
String headers = BufferUtil.toString(header);
assertThat(headers, containsString("Connection: other, close\r\n"));
assertThat(headers, not(containsString("keep-alive")));
}
}

18
jetty-jaspi/src/main/java/org/eclipse/jetty/security/jaspi/modules/FormAuthModule.java
View File

@ -35,6 +35,7 @@ import javax.servlet.http.HttpSession;
import org.eclipse.jetty.security.authentication.DeferredAuthentication;
import org.eclipse.jetty.security.authentication.LoginCallbackImpl;
import org.eclipse.jetty.security.authentication.SessionAuthentication;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.UserIdentity;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
@ -132,7 +133,6 @@ public class FormAuthModule extends BaseAuthModule
@Override
public AuthStatus validateRequest(MessageInfo messageInfo, Subject clientSubject, Subject serviceSubject) throws AuthException
{
HttpServletRequest request = (HttpServletRequest)messageInfo.getRequestMessage();
HttpServletResponse response = (HttpServletResponse)messageInfo.getResponseMessage();
String uri = request.getRequestURI();
@ -173,7 +173,7 @@ public class FormAuthModule extends BaseAuthModule
}
response.setContentLength(0);
response.sendRedirect(response.encodeRedirectURL(nuri));
Request.getBaseRequest(request).getResponse().sendRedirect(HttpServletResponse.SC_MOVED_TEMPORARILY, nuri, true);
return AuthStatus.SEND_CONTINUE;
}
// not authenticated
@ -187,7 +187,8 @@ public class FormAuthModule extends BaseAuthModule
else
{
response.setContentLength(0);
response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)));
Request.getBaseRequest(request).getResponse().sendRedirect(HttpServletResponse.SC_MOVED_TEMPORARILY,
response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)), true);
}
// TODO is this correct response if isMandatory false??? Can
// that occur?
@ -229,14 +230,13 @@ public class FormAuthModule extends BaseAuthModule
}
response.setContentLength(0);
response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)));
Request.getBaseRequest(request).getResponse().sendRedirect(
HttpServletResponse.SC_MOVED_TEMPORARILY,
response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)),
true);
return AuthStatus.SEND_CONTINUE;
}
catch (IOException e)
{
throw new AuthException(e.getMessage());
}
catch (UnsupportedCallbackException e)
catch (IOException | UnsupportedCallbackException e)
{
throw new AuthException(e.getMessage());
}

14
jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java
View File

@ -30,7 +30,6 @@ import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.security.LoginService;
import org.eclipse.jetty.security.ServerAuthException;
@ -300,7 +299,7 @@ public class OpenIdAuthenticator extends LoginAuthenticator
LOG.debug("authenticated {}->{}", openIdAuth, nuri);
response.setContentLength(0);
baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), nuri);
baseResponse.sendRedirect(nuri, true);
return openIdAuth;
}
}
@ -392,7 +391,7 @@ public class OpenIdAuthenticator extends LoginAuthenticator
String challengeUri = getChallengeUri(request);
if (LOG.isDebugEnabled())
LOG.debug("challenge {}->{}", session.getId(), challengeUri);
baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), challengeUri);
baseResponse.sendRedirect(challengeUri, true);
return Authentication.SEND_CONTINUE;
}
@ -436,10 +435,9 @@ public class OpenIdAuthenticator extends LoginAuthenticator
{
String query = URIUtil.addQueries(ERROR_PARAMETER + "=" + UrlEncoded.encodeString(message), _errorQuery);
redirectUri = URIUtil.addPathQuery(URIUtil.addPaths(request.getContextPath(), _errorPath), query);
baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), redirectUri);
}
baseResponse.sendRedirect(getRedirectCode(baseRequest.getHttpVersion()), redirectUri);
baseResponse.sendRedirect(redirectUri, true);
}
}
@ -461,12 +459,6 @@ public class OpenIdAuthenticator extends LoginAuthenticator
return pathInContext != null && (pathInContext.equals(_errorPath));
}
private static int getRedirectCode(HttpVersion httpVersion)
{
return (httpVersion.getVersion() < HttpVersion.HTTP_1_1.getVersion()
? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
}
private String getRedirectUri(HttpServletRequest request)
{
final StringBuffer redirectUri = new StringBuffer(128);

5
jetty-security/src/main/java/org/eclipse/jetty/security/ConstraintSecurityHandler.java
View File

@ -641,7 +641,8 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
if (dataConstraint == null || dataConstraint == UserDataConstraint.None)
return true;
HttpConfiguration httpConfig = Request.getBaseRequest(request).getHttpChannel().getHttpConfiguration();
Request baseRequest = Request.getBaseRequest(request);
HttpConfiguration httpConfig = baseRequest.getHttpChannel().getHttpConfiguration();
if (dataConstraint == UserDataConstraint.Confidential || dataConstraint == UserDataConstraint.Integral)
{
@ -655,7 +656,7 @@ public class ConstraintSecurityHandler extends SecurityHandler implements Constr
String url = URIUtil.newURI(scheme, request.getServerName(), port, request.getRequestURI(), request.getQueryString());
response.setContentLength(0);
response.sendRedirect(url);
response.sendRedirect(url, true);
}
else
response.sendError(HttpStatus.FORBIDDEN_403, "!Secure");

10
jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java
View File

@ -35,7 +35,6 @@ import javax.servlet.http.HttpSession;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.security.ServerAuthException;
import org.eclipse.jetty.security.UserAuthentication;
@ -291,8 +290,7 @@ public class FormAuthenticator extends LoginAuthenticator
LOG.debug("authenticated {}->{}", formAuth, nuri);
response.setContentLength(0);
int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(nuri));
baseResponse.sendRedirect(response.encodeRedirectURL(nuri), true);
return formAuth;
}
@ -316,8 +314,7 @@ public class FormAuthenticator extends LoginAuthenticator
else
{
LOG.debug("auth failed {}->{}", username, _formErrorPage);
int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)));
baseResponse.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formErrorPage)), true);
}
return Authentication.SEND_FAILURE;
@ -410,8 +407,7 @@ public class FormAuthenticator extends LoginAuthenticator
else
{
LOG.debug("challenge {}->{}", session.getId(), _formLoginPage);
int redirectCode = (baseRequest.getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion() ? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER);
baseResponse.sendRedirect(redirectCode, response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)));
baseResponse.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(), _formLoginPage)), true);
}
return Authentication.SEND_CONTINUE;
}

124
jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
View File

@ -77,6 +77,7 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.in;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
@ -468,9 +469,6 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/noauth/info HTTP/1.0\r\n\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 200 OK"));
scenarios.add(Arguments.of(
new Scenario(
"GET /ctx/forbid/info HTTP/1.0\r\n\r\n",
@ -478,9 +476,6 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/forbid/info HTTP/1.0\r\n\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 403 Forbidden"));
scenarios.add(Arguments.of(
new Scenario(
"GET /ctx/auth/info HTTP/1.0\r\n\r\n",
@ -493,9 +488,39 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
// assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
scenarios.add(Arguments.of(
new Scenario(
"POST /ctx/auth/info HTTP/1.1\r\n" +
"Host: test\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"0123456789",
HttpStatus.UNAUTHORIZED_401,
(response) ->
{
String authHeader = response.get(HttpHeader.WWW_AUTHENTICATE);
assertThat(response.toString(), authHeader, containsString("basic realm=\"TestRealm\""));
assertThat(response.get(HttpHeader.CONNECTION), nullValue());
}
)
));
scenarios.add(Arguments.of(
new Scenario(
"POST /ctx/auth/info HTTP/1.1\r\n" +
"Host: test\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"012345",
HttpStatus.UNAUTHORIZED_401,
(response) ->
{
String authHeader = response.get(HttpHeader.WWW_AUTHENTICATE);
assertThat(response.toString(), authHeader, containsString("basic realm=\"TestRealm\""));
assertThat(response.get(HttpHeader.CONNECTION), is("close"));
}
)
));
scenarios.add(Arguments.of(
new Scenario(
@ -511,12 +536,6 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" +
// "Authorization: Basic " + authBase64("user:wrong") + "\r\n" +
// "\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
// assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
scenarios.add(Arguments.of(
new Scenario(
"GET /ctx/auth/info HTTP/1.0\r\n" +
@ -526,10 +545,16 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" +
// "Authorization: Basic " + authBase64("user:password") + "\r\n" +
// "\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 200 OK"));
scenarios.add(Arguments.of(
new Scenario(
"POST /ctx/auth/info HTTP/1.0\r\n" +
"Content-Length: 10\r\n" +
"Authorization: Basic " + authBase64("user:password") + "\r\n" +
"\r\n" +
"0123456789",
HttpStatus.OK_200
)
));
// == test admin
scenarios.add(Arguments.of(
@ -544,10 +569,6 @@ public class ConstraintTest
)
));
// rawResponse = _connector.getResponse("GET /ctx/admin/info HTTP/1.0\r\n\r\n");
// assertThat(rawResponse, startsWith("HTTP/1.1 401 Unauthorized"));
// assertThat(rawResponse, containsString("WWW-Authenticate: basic realm=\"TestRealm\""));
scenarios.add(Arguments.of(
new Scenario(
"GET /ctx/admin/info HTTP/1.0\r\n" +
@ -1007,6 +1028,63 @@ public class ConstraintTest
assertThat(response, containsString("!role"));
}
@Test
public void testNonFormPostRedirectHttp10() throws Exception
{
_security.setAuthenticator(new FormAuthenticator("/testLoginPage", "/testErrorPage", false));
_server.start();
String response = _connector.getResponse("POST /ctx/auth/info HTTP/1.0\r\n" +
"Content-Type: text/plain\r\n" +
"Connection: keep-alive\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"0123456789\r\n");
assertThat(response, containsString(" 302 Found"));
assertThat(response, containsString("/ctx/testLoginPage"));
assertThat(response, not(containsString("Connection: close")));
assertThat(response, containsString("Connection: keep-alive"));
response = _connector.getResponse("POST /ctx/auth/info HTTP/1.0\r\n" +
"Host: localhost\r\n" +
"Content-Type: text/plain\r\n" +
"Connection: keep-alive\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"012345\r\n");
assertThat(response, containsString(" 302 Found"));
assertThat(response, containsString("/ctx/testLoginPage"));
assertThat(response, not(containsString("Connection: keep-alive")));
}
@Test
public void testNonFormPostRedirectHttp11() throws Exception
{
_security.setAuthenticator(new FormAuthenticator("/testLoginPage", "/testErrorPage", false));
_server.start();
String response = _connector.getResponse("POST /ctx/auth/info HTTP/1.1\r\n" +
"Host: test\r\n" +
"Content-Type: text/plain\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"0123456789\r\n");
assertThat(response, containsString(" 303 See Other"));
assertThat(response, containsString("/ctx/testLoginPage"));
assertThat(response, not(containsString("Connection: close")));
response = _connector.getResponse("POST /ctx/auth/info HTTP/1.1\r\n" +
"Host: test\r\n" +
"Host: localhost\r\n" +
"Content-Type: text/plain\r\n" +
"Content-Length: 10\r\n" +
"\r\n" +
"012345\r\n");
assertThat(response, containsString(" 303 See Other"));
assertThat(response, containsString("/ctx/testLoginPage"));
assertThat(response, containsString("Connection: close"));
}
@Test
public void testFormNoCookies() throws Exception
{

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

@ -31,14 +31,18 @@ import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.HttpGenerator;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpVersion;
@ -54,6 +58,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.SharedBlockingCallback.Blocker;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Scheduler;
@ -406,7 +411,16 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
// the following is needed as you cannot trust the response code and reason
// as those could have been modified after calling sendError
Integer code = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
_response.setStatus(code != null ? code : HttpStatus.INTERNAL_SERVER_ERROR_500);
if (code == null)
code = HttpStatus.INTERNAL_SERVER_ERROR_500;
_response.setStatus(code);
// The handling of the original dispatch failed and we are now going to either generate
// and error response ourselves or dispatch for an error page. If there is content left over
// from the failed dispatch, then we try to consume it here and if we fail we add a
// Connection:close. This can't be deferred to COMPLETE as the response will be committed
// by then.
ensureConsumeAllOrNotPersistent();
ContextHandler.Context context = (ContextHandler.Context)_request.getAttribute(ErrorHandler.ERROR_CONTEXT);
ErrorHandler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler());
@ -492,10 +506,18 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
case COMPLETE:
{
if (!_response.isCommitted() && !_request.isHandled() && !_response.getHttpOutput().isClosed())
if (!_response.isCommitted())
{
_response.sendError(HttpStatus.NOT_FOUND_404);
break;
if (!_request.isHandled() && !_response.getHttpOutput().isClosed())
{
// The request was not actually handled
_response.sendError(HttpStatus.NOT_FOUND_404);
break;
}
// Indicate Connection:close if we can't consume all.
if (_response.getStatus() >= 200)
ensureConsumeAllOrNotPersistent();
}
// RFC 7230, section 3.3.
@ -511,12 +533,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
break;
}
}
// TODO Currently a blocking/aborting consumeAll is done in the handling of the TERMINATED
// TODO Action triggered by the completed callback below. It would be possible to modify the
// TODO callback to do a non-blocking consumeAll at this point and only call completed when
// TODO that is done.
// Set a close callback on the HttpOutput to make it an async callback
_response.completeOutput(Callback.from(() -> _state.completed(null), _state::completed));
@ -545,6 +562,66 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
return !suspended;
}
public void ensureConsumeAllOrNotPersistent()
{
switch (_request.getHttpVersion())
{
case HTTP_1_0:
if (_request.getHttpInput().consumeAll())
return;
// Remove any keep-alive value in Connection headers
_response.getHttpFields().computeField(HttpHeader.CONNECTION, (h, fields) ->
{
if (fields == null || fields.isEmpty())
return null;
String v = fields.stream()
.flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)))
.collect(Collectors.joining(", "));
if (StringUtil.isEmpty(v))
return null;
return new HttpField(HttpHeader.CONNECTION, v);
});
break;
case HTTP_1_1:
if (_request.getHttpInput().consumeAll())
return;
// Add close value to Connection headers
_response.getHttpFields().computeField(HttpHeader.CONNECTION, (h, fields) ->
{
if (fields == null || fields.isEmpty())
return HttpConnection.CONNECTION_CLOSE;
if (fields.stream().anyMatch(f -> f.contains(HttpHeaderValue.CLOSE.asString())))
{
if (fields.size() == 1)
{
HttpField f = fields.get(0);
if (HttpConnection.CONNECTION_CLOSE.equals(f))
return f;
}
return new HttpField(HttpHeader.CONNECTION, fields.stream()
.flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s)))
.collect(Collectors.joining(", ")));
}
return new HttpField(HttpHeader.CONNECTION,
Stream.concat(fields.stream()
.flatMap(field -> Stream.of(field.getValues()).filter(s -> !HttpHeaderValue.KEEP_ALIVE.is(s))),
Stream.of(HttpHeaderValue.CLOSE.asString()))
.collect(Collectors.joining(", ")));
});
break;
default:
break;
}
}
private void dispatch(DispatcherType type, Dispatchable dispatchable) throws IOException, ServletException
{
try

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

@ -904,8 +904,6 @@ public class HttpChannelState
default:
throw new IllegalStateException(getStatusStringLocked());
}
if (_outputState != OutputState.OPEN)
throw new IllegalStateException("Response is " + _outputState);
response.setStatus(code);
response.errorClose();

26
jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
View File

@ -408,28 +408,12 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
// close to seek EOF
_parser.close();
}
else if (_parser.inContentState() && _generator.isPersistent())
// else abort if we can't consume all
else if (_generator.isPersistent() && !_input.consumeAll())
{
// Try to progress without filling.
parseRequestBuffer();
if (_parser.inContentState())
{
// If we are async, then we have problems to complete neatly
if (_input.isAsync())
{
if (LOG.isDebugEnabled())
LOG.debug("{}unconsumed input while async {}", _parser.isChunking() ? "Possible " : "", this);
_channel.abort(new IOException("unconsumed input"));
}
else
{
if (LOG.isDebugEnabled())
LOG.debug("{}unconsumed input {}", _parser.isChunking() ? "Possible " : "", this);
// Complete reading the request
if (!_input.consumeAll())
_channel.abort(new IOException("unconsumed input"));
}
}
if (LOG.isDebugEnabled())
LOG.debug("unconsumed input {} {}", this, _parser);
_channel.abort(new IOException("unconsumed input"));
}
// Reset the channel, parsers and generator

75
jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java
View File

@ -154,13 +154,14 @@ public class HttpInput extends ServletInputStream implements Runnable
{
synchronized (_inputQ)
{
if (_content != null)
_content.failed(null);
Throwable failure = fail(_intercepted, null);
_intercepted = null;
failure = fail(_content, failure);
_content = null;
Content item = _inputQ.poll();
while (item != null)
{
item.failed(null);
failure = fail(item, failure);
item = _inputQ.poll();
}
_listener = null;
@ -176,6 +177,17 @@ public class HttpInput extends ServletInputStream implements Runnable
}
}
private Throwable fail(Content content, Throwable failure)
{
if (content != null)
{
if (failure == null)
failure = new IOException("unconsumed input");
content.failed(failure);
}
return failure;
}
/**
* @return The current Interceptor, or null if none set
*/
@ -670,31 +682,52 @@ public class HttpInput extends ServletInputStream implements Runnable
return addContent(EOF_CONTENT);
}
/**
* Consume all available content without blocking.
* Raw content is counted in the {@link #getContentReceived()} statistics, but
* is not intercepted nor counted in the {@link #getContentConsumed()} statistics
* @return True if EOF was reached, false otherwise.
*/
public boolean consumeAll()
{
synchronized (_inputQ)
while (true)
{
try
synchronized (_inputQ)
{
while (true)
if (_intercepted != null)
{
Content item = nextContent();
if (item == null)
break; // Let's not bother blocking
skip(item, item.remaining());
_intercepted.skip(_intercepted.remaining());
consume(_intercepted);
}
if (isFinished())
return !isError();
_state = EARLY_EOF;
return false;
}
catch (Throwable e)
{
LOG.debug(e);
_state = new ErrorState(e);
return false;
if (_content != null)
{
_content.skip(_content.remaining());
consume(_content);
}
Content content = _inputQ.poll();
while (content != null)
{
consume(content);
content = _inputQ.poll();
}
if (_state instanceof EOFState)
return !(_state instanceof ErrorState);
try
{
produceContent();
if (_content == null && _intercepted == null && _inputQ.isEmpty())
return false;
}
catch (Throwable e)
{
LOG.debug(e);
_state = new ErrorState(e);
return false;
}
}
}
}

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

@ -500,7 +500,37 @@ public class Response implements HttpServletResponse
*/
public void sendRedirect(int code, String location) throws IOException
{
if ((code < HttpServletResponse.SC_MULTIPLE_CHOICES) || (code >= HttpServletResponse.SC_BAD_REQUEST))
sendRedirect(code, location, false);
}
/**
* Sends a response with a HTTP version appropriate 30x redirection.
*
* @param location the location to send in {@code Location} headers
* @param consumeAll if True, consume any HTTP/1 request input before doing the redirection. If the input cannot
* be consumed without blocking, then add a `Connection: close` header to the response.
* @throws IOException if unable to send the redirect
*/
public void sendRedirect(String location, boolean consumeAll) throws IOException
{
sendRedirect(getHttpChannel().getRequest().getHttpVersion().getVersion() < HttpVersion.HTTP_1_1.getVersion()
? HttpServletResponse.SC_MOVED_TEMPORARILY : HttpServletResponse.SC_SEE_OTHER, location, consumeAll);
}
/**
* Sends a response with a given redirection code.
*
* @param code the redirect status code
* @param location the location to send in {@code Location} headers
* @param consumeAll if True, consume any HTTP/1 request input before doing the redirection. If the input cannot
* be consumed without blocking, then add a `Connection: close` header to the response.
* @throws IOException if unable to send the redirect
*/
public void sendRedirect(int code, String location, boolean consumeAll) throws IOException
{
if (consumeAll)
getHttpChannel().ensureConsumeAllOrNotPersistent();
if (!HttpStatus.isRedirection(code))
throw new IllegalArgumentException("Not a 3xx redirect code");
if (!isMutable())

9
jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
View File

@ -1240,10 +1240,11 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
{
// context request must end with /
baseRequest.setHandled(true);
if (baseRequest.getQueryString() != null)
response.sendRedirect(baseRequest.getRequestURI() + "/?" + baseRequest.getQueryString());
else
response.sendRedirect(baseRequest.getRequestURI() + "/");
String queryString = baseRequest.getQueryString();
baseRequest.getResponse().sendRedirect(
HttpServletResponse.SC_MOVED_TEMPORARILY,
baseRequest.getRequestURI() + (queryString == null ? "/" : ("/?" + queryString)),
true);
return false;
}

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

@ -62,7 +62,7 @@ public class SecuredRedirectHandler extends AbstractHandler
String url = URIUtil.newURI(scheme, baseRequest.getServerName(), port, baseRequest.getRequestURI(), baseRequest.getQueryString());
response.setContentLength(0);
response.sendRedirect(url);
baseRequest.getResponse().sendRedirect(HttpServletResponse.SC_MOVED_TEMPORARILY, url, true);
}
else
{

6
jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpInputInterceptor.java
View File

@ -55,6 +55,12 @@ public class GzipHttpInputInterceptor implements HttpInput.Interceptor, Destroya
{
_decoder.release(chunk);
}
@Override
public void failed(Throwable x)
{
_decoder.release(chunk);
}
};
}

1
jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java
View File

@ -328,6 +328,7 @@ public class AsyncRequestReadTest
BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
assertThat(in.readLine(), containsString("HTTP/1.1 200 OK"));
assertThat(in.readLine(), containsString("Connection: close"));
assertThat(in.readLine(), containsString("Content-Length:"));
assertThat(in.readLine(), containsString("Server:"));
in.readLine();

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

@ -233,6 +233,90 @@ public class ErrorHandlerTest
assertContent(response);
}
@Test
public void test404PostHttp10() throws Exception
{
String rawResponse = connector.getResponse(
"POST / HTTP/1.0\r\n" +
"Host: Localhost\r\n" +
"Accept: text/html\r\n" +
"Content-Length: 10\r\n" +
"Connection: keep-alive\r\n" +
"\r\n" +
"0123456789");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(404));
assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
assertThat(response.get(HttpHeader.CONNECTION), is("keep-alive"));
assertContent(response);
}
@Test
public void test404PostHttp11() throws Exception
{
String rawResponse = connector.getResponse(
"POST / HTTP/1.1\r\n" +
"Host: Localhost\r\n" +
"Accept: text/html\r\n" +
"Content-Length: 10\r\n" +
"Connection: keep-alive\r\n" + // This is not need by HTTP/1.1 but sometimes sent anyway
"\r\n" +
"0123456789");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(404));
assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
assertThat(response.getField(HttpHeader.CONNECTION), nullValue());
assertContent(response);
}
@Test
public void test404PostCantConsumeHttp10() throws Exception
{
String rawResponse = connector.getResponse(
"POST / HTTP/1.0\r\n" +
"Host: Localhost\r\n" +
"Accept: text/html\r\n" +
"Content-Length: 100\r\n" +
"Connection: keep-alive\r\n" +
"\r\n" +
"0123456789");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(404));
assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
assertThat(response.getField(HttpHeader.CONNECTION), nullValue());
assertContent(response);
}
@Test
public void test404PostCantConsumeHttp11() throws Exception
{
String rawResponse = connector.getResponse(
"POST / HTTP/1.1\r\n" +
"Host: Localhost\r\n" +
"Accept: text/html\r\n" +
"Content-Length: 100\r\n" +
"Connection: keep-alive\r\n" + // This is not need by HTTP/1.1 but sometimes sent anyway
"\r\n" +
"0123456789");
HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(404));
assertThat(response.getField(HttpHeader.CONTENT_LENGTH).getIntValue(), greaterThan(0));
assertThat(response.get(HttpHeader.CONTENT_TYPE), containsString("text/html;charset=ISO-8859-1"));
assertThat(response.getField(HttpHeader.CONNECTION).getValue(), is("close"));
assertContent(response);
}
@Test
public void testMoreSpecificAccept() throws Exception
{

6
jetty-server/src/test/java/org/eclipse/jetty/server/HttpInputTest.java
View File

@ -498,7 +498,8 @@ public class HttpInputTest
assertThat(_in.read(), equalTo((int)'A'));
assertFalse(_in.consumeAll());
assertThat(_in.getContentConsumed(), equalTo(8L));
assertThat(_in.getContentConsumed(), equalTo(1L));
assertThat(_in.getContentReceived(), equalTo(8L));
assertThat(_history.poll(), equalTo("Content succeeded AB"));
assertThat(_history.poll(), equalTo("Content succeeded CD"));
@ -520,7 +521,8 @@ public class HttpInputTest
assertThat(_in.read(), equalTo((int)'A'));
assertTrue(_in.consumeAll());
assertThat(_in.getContentConsumed(), equalTo(8L));
assertThat(_in.getContentConsumed(), equalTo(1L));
assertThat(_in.getContentReceived(), equalTo(8L));
assertThat(_history.poll(), equalTo("Content succeeded AB"));
assertThat(_history.poll(), equalTo("Content succeeded CD"));

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

@ -1293,6 +1293,78 @@ public class ResponseTest
output.flush();
}
@Test
public void testEnsureConsumeAllOrNotPersistentHttp10() throws Exception
{
Response response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_0);
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), nullValue());
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_0);
response.setHeader(HttpHeader.CONNECTION, "keep-alive");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), nullValue());
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_0);
response.setHeader(HttpHeader.CONNECTION, "before");
response.getHttpFields().add(HttpHeader.CONNECTION, "foo, keep-alive, bar");
response.getHttpFields().add(HttpHeader.CONNECTION, "after");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, foo, bar, after"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_0);
response.setHeader(HttpHeader.CONNECTION, "close");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
}
@Test
public void testEnsureConsumeAllOrNotPersistentHttp11() throws Exception
{
Response response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.setHeader(HttpHeader.CONNECTION, "keep-alive");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.setHeader(HttpHeader.CONNECTION, "close");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("close"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.setHeader(HttpHeader.CONNECTION, "before, close, after");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, close, after"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.setHeader(HttpHeader.CONNECTION, "before");
response.getHttpFields().add(HttpHeader.CONNECTION, "middle, close");
response.getHttpFields().add(HttpHeader.CONNECTION, "after");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("before, middle, close, after"));
response = getResponse();
response.getHttpChannel().getRequest().setHttpVersion(HttpVersion.HTTP_1_1);
response.setHeader(HttpHeader.CONNECTION, "one");
response.getHttpFields().add(HttpHeader.CONNECTION, "two");
response.getHttpFields().add(HttpHeader.CONNECTION, "three");
response.getHttpChannel().ensureConsumeAllOrNotPersistent();
assertThat(response.getHttpFields().get(HttpHeader.CONNECTION), is("one, two, three, close"));
}
private Response getResponse()
{
_channel.recycle();

166
tests/test-integration/src/test/java/org/eclipse/jetty/test/GzipWithSendErrorTest.java Normal file
View File

@ -0,0 +1,166 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.test;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.net.URI;
import java.util.zip.GZIPOutputStream;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.util.BytesContentProvider;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.component.LifeCycle;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class GzipWithSendErrorTest
{
private Server server;
private HttpClient client;
@BeforeEach
public void setup() throws Exception
{
server = new Server();
ServerConnector connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);
GzipHandler gzipHandler = new GzipHandler();
gzipHandler.setInflateBufferSize(4096);
ServletContextHandler contextHandler = new ServletContextHandler();
contextHandler.setContextPath("/");
contextHandler.addServlet(PostServlet.class, "/submit");
contextHandler.addServlet(FailServlet.class, "/fail");
gzipHandler.setHandler(contextHandler);
server.setHandler(gzipHandler);
server.start();
client = new HttpClient();
client.start();
}
@AfterEach
public void teardown()
{
LifeCycle.stop(client);
LifeCycle.stop(server);
}
/**
* Make 3 requests on the same connection.
* <p>
* Normal POST with 200 response, POST which results in 400, POST with 200 response.
* </p>
*/
@Test
public void testGzipNormalErrorNormal() throws Exception
{
URI serverURI = server.getURI();
ContentResponse response;
response = client.newRequest(serverURI.resolve("/submit"))
.method(HttpMethod.POST)
.header(HttpHeader.CONTENT_ENCODING, "gzip")
.header(HttpHeader.ACCEPT_ENCODING, "gzip")
.content(new BytesContentProvider("text/plain", compressed("normal-A")))
.send();
assertEquals(200, response.getStatus(), "Response status on /submit (normal-A)");
assertEquals("normal-A", response.getContentAsString(), "Response content on /submit (normal-A)");
response = client.newRequest(serverURI.resolve("/fail"))
.method(HttpMethod.POST)
.header(HttpHeader.CONTENT_ENCODING, "gzip")
.header(HttpHeader.ACCEPT_ENCODING, "gzip")
.content(new BytesContentProvider("text/plain", compressed("normal-B")))
.send();
assertEquals(400, response.getStatus(), "Response status on /fail (normal-B)");
assertThat("Response content on /fail (normal-B)", response.getContentAsString(), containsString("<title>Error 400 Bad Request</title>"));
response = client.newRequest(serverURI.resolve("/submit"))
.method(HttpMethod.POST)
.header(HttpHeader.CONTENT_ENCODING, "gzip")
.header(HttpHeader.ACCEPT_ENCODING, "gzip")
.content(new BytesContentProvider("text/plain", compressed("normal-C")))
.send();
assertEquals(200, response.getStatus(), "Response status on /submit (normal-C)");
assertEquals("normal-C", response.getContentAsString(), "Response content on /submit (normal-C)");
}
private byte[] compressed(String content) throws IOException
{
try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
GZIPOutputStream gzipOut = new GZIPOutputStream(baos))
{
gzipOut.write(content.getBytes(UTF_8));
gzipOut.finish();
return baos.toByteArray();
}
}
public static class PostServlet extends HttpServlet
{
@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
resp.setCharacterEncoding("utf-8");
resp.setContentType("text/plain");
resp.setHeader("X-Servlet", req.getServletPath());
String reqBody = IO.toString(req.getInputStream(), UTF_8);
resp.getWriter().append(reqBody);
}
}
public static class FailServlet extends HttpServlet
{
@Override
protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException
{
resp.setHeader("X-Servlet", req.getServletPath());
// intentionally do not read request body here.
resp.sendError(400);
}
}
}