Skip to content

Commit

Permalink
Improved handling of 100 Continue (#12113)
Browse files Browse the repository at this point in the history
* Now `HttpClient` removed the `Expect` header if there is no request content.
* Changed AbstractProxyServlet and ProxyHandler check for request content: now the Content-Type header is not taken into consideration.
* Now the server avoids sending the 100 Continue response if there is no request content.
* Now the request body is not defaulted if missing, but just kept null.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet authored Aug 13, 2024
1 parent 8277051 commit fc9cbda
Show file tree
Hide file tree
Showing 11 changed files with 261 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,14 @@ public String getName()
@Override
public boolean accept(Request request, Response response)
{
boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
boolean expect100 = request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
boolean handled100 = request.getAttributes().containsKey(ATTRIBUTE);
return (is100 || expect100) && !handled100;
if (handled100)
return false;
boolean is100 = response.getStatus() == HttpStatus.CONTINUE_100;
if (is100)
return true;
// Also handle non-100 responses, because we need to complete the request to complete the whole exchange.
return request.getHeaders().contains(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import org.eclipse.jetty.client.Authentication;
import org.eclipse.jetty.client.AuthenticationStore;
import org.eclipse.jetty.client.BytesRequestContent;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpProxy;
import org.eclipse.jetty.client.HttpRequestException;
Expand All @@ -35,6 +34,7 @@
import org.eclipse.jetty.io.CyclicTimeouts;
import org.eclipse.jetty.util.Attachable;
import org.eclipse.jetty.util.NanoTime;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.thread.AutoLock;
import org.eclipse.jetty.util.thread.Scheduler;
import org.slf4j.Logger;
Expand Down Expand Up @@ -146,7 +146,7 @@ protected void normalizeRequest(HttpRequest request)

// Make sure the path is there
String path = request.getPath();
if (path.trim().length() == 0)
if (StringUtil.isBlank(path))
{
path = "/";
request.path(path);
Expand Down Expand Up @@ -191,22 +191,15 @@ protected void normalizeRequest(HttpRequest request)

// Add content headers.
Request.Content content = request.getBody();
if (content == null)
{
request.body(new BytesRequestContent());
}
else
if (content != null)
{
if (!headers.contains(HttpHeader.CONTENT_TYPE))
{
String contentType = content.getContentType();
if (contentType == null)
contentType = getHttpClient().getDefaultRequestContentType();
if (contentType != null)
{
HttpField field = new HttpField(HttpHeader.CONTENT_TYPE, contentType);
request.addHeader(field);
}
request.addHeader(new HttpField(HttpHeader.CONTENT_TYPE, contentType));
}
long contentLength = content.getLength();
if (contentLength >= 0)
Expand All @@ -215,6 +208,9 @@ protected void normalizeRequest(HttpRequest request)
request.addHeader(new HttpField.LongValueHttpField(HttpHeader.CONTENT_LENGTH, contentLength));
}
}
// RFC 9110, section 10.1.1.
if (content == null || content.getLength() == 0)
request.headers(h -> h.remove(HttpHeader.EXPECT));

// Cookies.
StringBuilder cookies = convertCookies(request.getCookies(), null);
Expand Down Expand Up @@ -243,7 +239,7 @@ private StringBuilder convertCookies(List<HttpCookie> cookies, StringBuilder bui
{
if (builder == null)
builder = new StringBuilder();
if (builder.length() > 0)
if (!builder.isEmpty())
builder.append("; ");
builder.append(cookie.getName()).append("=").append(cookie.getValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ protected Action process() throws Throwable
action.run();

// Read the request content.
chunk = content.read();
chunk = content != null ? content.read() : Content.Chunk.EOF;
}
if (LOG.isDebugEnabled())
LOG.debug("Content {} for {}", chunk, request);
Expand All @@ -539,6 +539,7 @@ protected Action process() throws Throwable
{
// No content after the headers, demand.
demanded = true;
assert content != null;
content.demand(this::succeeded);
return Action.SCHEDULED;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,12 @@ protected void addForwardedHeader(Request clientToProxyRequest, org.eclipse.jett

private boolean hasContent(Request clientToProxyRequest)
{
if (clientToProxyRequest.getLength() > 0)
long contentLength = clientToProxyRequest.getLength();
if (contentLength == 0)
return false;
if (contentLength > 0)
return true;
HttpFields headers = clientToProxyRequest.getHeaders();
return headers.get(HttpHeader.CONTENT_TYPE) != null ||
headers.get(HttpHeader.TRANSFER_ENCODING) != null;
return clientToProxyRequest.getHeaders().get(HttpHeader.TRANSFER_ENCODING) != null;
}

private boolean expects100Continue(Request clientToProxyRequest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1117,8 +1117,6 @@ public String toString()
*/
public static class ChannelResponse implements Response, Callback
{
private static final CompletableFuture<Void> UNEXPECTED_100_CONTINUE = CompletableFuture.failedFuture(new IllegalStateException("100 not expected"));
private static final CompletableFuture<Void> COMMITTED_100_CONTINUE = CompletableFuture.failedFuture(new IllegalStateException("Committed"));
private final ChannelRequest _request;
private final ResponseHttpFields _httpFields;
protected int _status;
Expand Down Expand Up @@ -1408,12 +1406,14 @@ public CompletableFuture<Void> writeInterim(int status, HttpFields headers)
if (status == HttpStatus.CONTINUE_100)
{
if (!httpChannelState._expects100Continue)
return UNEXPECTED_100_CONTINUE;
return CompletableFuture.failedFuture(new IllegalStateException("100 not expected"));
if (_request.getLength() == 0)
return CompletableFuture.completedFuture(null);
httpChannelState._expects100Continue = false;
}

if (_httpFields.isCommitted())
return status == HttpStatus.CONTINUE_100 ? COMMITTED_100_CONTINUE : CompletableFuture.failedFuture(new IllegalStateException("Committed"));
return CompletableFuture.failedFuture(new IllegalStateException("Committed"));
if (_writeCallback != null)
return CompletableFuture.failedFuture(new WritePendingException());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -657,21 +657,7 @@ public static int indexOfControlChars(String str)
*/
public static boolean isBlank(String str)
{
if (str == null)
{
return true;
}
int len = str.length();
for (int i = 0; i < len; i++)
{
if (!Character.isWhitespace(str.codePointAt(i)))
{
// found a non-whitespace, we can stop searching now
return false;
}
}
// only whitespace
return true;
return str == null || str.isBlank();
}

/**
Expand Down Expand Up @@ -727,21 +713,7 @@ public static int getLength(String s)
*/
public static boolean isNotBlank(String str)
{
if (str == null)
{
return false;
}
int len = str.length();
for (int i = 0; i < len; i++)
{
if (!Character.isWhitespace(str.codePointAt(i)))
{
// found a non-whitespace, we can stop searching now
return true;
}
}
// only whitespace
return false;
return !isBlank(str);
}

public static boolean isHex(String str, int offset, int length)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,12 @@ protected void onProxyRewriteFailed(HttpServletRequest clientRequest, HttpServle

protected boolean hasContent(HttpServletRequest clientRequest)
{
return clientRequest.getContentLength() > 0 ||
clientRequest.getContentType() != null ||
clientRequest.getHeader(HttpHeader.TRANSFER_ENCODING.asString()) != null;
long contentLength = clientRequest.getContentLengthLong();
if (contentLength == 0)
return false;
if (contentLength > 0)
return true;
return clientRequest.getHeader(HttpHeader.TRANSFER_ENCODING.asString()) != null;
}

protected boolean expects100Continue(HttpServletRequest request)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import java.io.PrintWriter;
import java.io.Writer;
import java.net.ConnectException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.SocketChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -80,6 +82,7 @@
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
Expand Down Expand Up @@ -1703,4 +1706,80 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}

@ParameterizedTest
@MethodSource("impls")
public void testExpect100ContinueContentLengthZero(Class<? extends ProxyServlet> proxyServletClass) throws Exception
{
testExpect100ContinueNoRequestContent(proxyServletClass, false);
}

@ParameterizedTest
@MethodSource("impls")
public void testExpect100ContinueEmptyChunkedContent(Class<? extends ProxyServlet> proxyServletClass) throws Exception
{
testExpect100ContinueNoRequestContent(proxyServletClass, true);
}

private void testExpect100ContinueNoRequestContent(Class<? extends ProxyServlet> proxyServletClass, boolean chunked) throws Exception
{
startServer(new HttpServlet()
{
@Override
protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException
{
// Send the 100 Continue.
ServletInputStream input = request.getInputStream();
// Echo the content.
IO.copy(input, response.getOutputStream());
}
});
startProxy(proxyServletClass);

String authority = "localhost:" + serverConnector.getLocalPort();
for (int i = 0; i < 50; i++)
{
try (SocketChannel client = SocketChannel.open(new InetSocketAddress("localhost", proxyConnector.getLocalPort())))
{
String request;
if (chunked)
{
request = """
POST http://$A/ HTTP/1.1
Host: $A
Expect: 100-Continue
Transfer-Encoding: chunked
0
""";
}
else
{
request = """
POST http://$A/ HTTP/1.1
Host: $A
Expect: 100-Continue
Content-Length: 0
""";
}
request = request.replace("$A", authority);
client.write(StandardCharsets.UTF_8.encode(request));

HttpTester.Input input = HttpTester.from(client);
HttpTester.Response response1 = HttpTester.parseResponse(input);
if (chunked)
{
assertEquals(HttpStatus.CONTINUE_100, response1.getStatus());
HttpTester.Response response2 = HttpTester.parseResponse(input);
assertEquals(HttpStatus.OK_200, response2.getStatus());
}
else
{
assertEquals(HttpStatus.OK_200, response1.getStatus());
}
}
}
}
}
Loading

0 comments on commit fc9cbda

Please sign in to comment.