Skip to content

Commit

Permalink
Implement #8057 103 Early Hint
Browse files Browse the repository at this point in the history 
updates from review
  • Loading branch information
@gregw
gregw committed May 26, 2022
1 parent de16b5b commit bfa8c65
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 83 deletions.
2 changes: 1 addition & 1 deletion jetty-http/src/main/java/org/eclipse/jetty/http/HttpGenerator.java
View file
Expand Up @@ -385,7 +385,7 @@ public Result generateResponse(MetaData.Response info, boolean head, ByteBuffer

// Handle 1xx and no content responses
int status = info.getStatus();
if (status >= 100 && status < 200)
if (HttpStatus.isInformational(status))
{
_noContentResponse = true;
switch (status)
Expand Down
2 changes: 1 addition & 1 deletion ...tp2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
View file
Expand Up @@ -98,7 +98,7 @@ public void sendHeaders(MetaData.Request request, MetaData.Response response, By
boolean isHeadRequest = HttpMethod.HEAD.is(request.getMethod());
boolean hasContent = BufferUtil.hasContent(content) && !isHeadRequest;
int status = response.getStatus();
boolean interimResponse = status >= HttpStatus.CONTINUE_100 && status < HttpStatus.OK_200 && status != HttpStatus.SWITCHING_PROTOCOLS_101;
boolean interimResponse = HttpStatus.isInformational(status) && status != HttpStatus.SWITCHING_PROTOCOLS_101;
if (interimResponse)
{
// Must not commit interim responses.
Expand Down
2 changes: 1 addition & 1 deletion ...-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpTransportOverHTTP3.java
View file
Expand Up @@ -71,7 +71,7 @@ private void sendHeaders(MetaData.Request request, MetaData.Response response, B
boolean isHeadRequest = HttpMethod.HEAD.is(request.getMethod());
boolean hasContent = BufferUtil.hasContent(content) && !isHeadRequest;
int status = response.getStatus();
boolean interimResponse = status >= HttpStatus.CONTINUE_100 && status < HttpStatus.OK_200 && status != HttpStatus.SWITCHING_PROTOCOLS_101;
boolean interimResponse = HttpStatus.isInformational(status) && status != HttpStatus.SWITCHING_PROTOCOLS_101;
if (interimResponse)
{
// Must not commit interim responses.
Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
View file
Expand Up @@ -1046,7 +1046,7 @@ protected boolean sendResponse(MetaData.Response response, ByteBuffer content, b

// wrap callback to process 100 responses
final int status = response.getStatus();
final Callback committed = (status < HttpStatus.OK_200 && status >= HttpStatus.CONTINUE_100)
final Callback committed = HttpStatus.isInformational(status)
? new Send100Callback(callback)
: new SendCallback(callback, content, true, complete);

Expand Down
15 changes: 7 additions & 8 deletions jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
View file
Expand Up @@ -470,6 +470,7 @@ public void sendError(int sc) throws IOException
* <p>In addition to the servlet standard handling, this method supports some additional codes:</p>
* <dl>
* <dt>102</dt><dd>Send a partial PROCESSING response and allow additional responses</dd>
* <dt>103</dt><dd>Send a partial EARLY_HINT response as per <a href="https://datatracker.ietf.org/doc/html/rfc8297">RFC8297</a></dd>
* <dt>-1</dt><dd>Abort the HttpChannel and close the connection/stream</dd>
* </dl>
* @param code The error code
Expand Down Expand Up @@ -501,9 +502,8 @@ public void sendError(int code, String message) throws IOException

/**
* Sends a 102-Processing response.
* If the connection is an HTTP connection, the version is 1.1 and the
* request has a Expect header starting with 102, then a 102 response is
* sent. This indicates that the request still be processed and real response
* If the request had an Expect header starting with 102, then
* a 102 response is sent. This indicates that the request still be processed and real response
* can still be sent. This method is called by sendError if it is passed 102.
*
* @throws IOException if unable to send the 102 response
Expand All @@ -518,11 +518,10 @@ public void sendProcessing() throws IOException
}

/**
* Sends a 102-Processing response.
* If the connection is an HTTP connection, the version is 1.1 and the
* request has a Expect header starting with 102, then a 102 response is
* sent. This indicates that the request still be processed and real response
* can still be sent. This method is called by sendError if it is passed 102.
* Sends a 103 Early Hint response.
*
* Send a 103 response as per <a href="https://datatracker.ietf.org/doc/html/rfc8297">RFC8297</a>
* This method is called by sendError if it is passed 103.
*
* @throws IOException if unable to send the 102 response
* @see javax.servlet.http.HttpServletResponse#sendError(int)
Expand Down
77 changes: 6 additions & 71 deletions ...http/client/IntermediateResponseTest.java → ...ttp/client/InformationalResponseTest.java
View file
Expand Up @@ -23,23 +23,18 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jetty.client.ContinueProtocolHandler;
import org.eclipse.jetty.client.HttpConversation;
import org.eclipse.jetty.client.HttpExchange;
import org.eclipse.jetty.client.HttpRequest;
import org.eclipse.jetty.client.ProtocolHandler;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.util.AsyncRequestContent;
import org.eclipse.jetty.client.util.BufferingResponseListener;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpHeaderValue;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.IO;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;
Expand All @@ -50,7 +45,7 @@
import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class IntermediateResponseTest extends AbstractTest<TransportScenario>
public class InformationalResponseTest extends AbstractTest<TransportScenario>
{
@Override
public void init(Transport transport) throws IOException
Expand All @@ -60,65 +55,6 @@ public void init(Transport transport) throws IOException
setScenario(new TransportScenario(transport));
}

@ParameterizedTest
@ArgumentsSource(TransportProvider.class)
public void test100Continue(Transport transport) throws Exception
{
init(transport);
scenario.start(new AbstractHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
jettyRequest.setHandled(true);
String body = IO.toString(request.getInputStream());
response.getOutputStream().print("read " + body.length());
}
});
long idleTimeout = 10000;
scenario.setRequestIdleTimeout(idleTimeout);

AsyncRequestContent content = new AsyncRequestContent()
{
@Override
public void demand()
{
super.demand();
}
};

scenario.client.getProtocolHandlers().put(new ContinueProtocolHandler()
{
@Override
protected void onContinue(org.eclipse.jetty.client.api.Request request)
{
super.onContinue(request);
content.offer(BufferUtil.toBuffer("Some content!"), Callback.from(content::close));
}
});

CountDownLatch complete = new CountDownLatch(1);
AtomicReference<Response> response = new AtomicReference<>();
BufferingResponseListener listener = new BufferingResponseListener()
{
@Override
public void onComplete(Result result)
{
response.set(result.getResponse());
complete.countDown();
}
};
scenario.client.POST(scenario.newURI())
.headers(headers -> headers.put(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE))
.body(content)
.timeout(10, TimeUnit.SECONDS)
.send(listener);

assertTrue(complete.await(10, TimeUnit.SECONDS));
assertThat(response.get().getStatus(), is(200));
assertThat(listener.getContentAsString(), is("read 13"));
}

@ParameterizedTest
@ArgumentsSource(TransportProvider.class)
public void test102Processing(Transport transport) throws Exception
Expand All @@ -127,7 +63,7 @@ public void test102Processing(Transport transport) throws Exception
scenario.start(new AbstractHandler()
{
@Override
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
{
jettyRequest.setHandled(true);
response.sendError(HttpStatus.PROCESSING_102);
Expand Down Expand Up @@ -161,7 +97,7 @@ public Response.Listener getResponseListener()
@Override
public void onSuccess(Response response)
{
org.eclipse.jetty.client.api.Request request = response.getRequest();
var request = response.getRequest();
HttpConversation conversation = ((HttpRequest)request).getConversation();
// Reset the conversation listeners, since we are going to receive another response code
conversation.updateResponseListeners(null);
Expand All @@ -175,7 +111,7 @@ public void onSuccess(Response response)
}
else
{
throw new IllegalStateException("should not have accepted");
response.abort(new IllegalStateException("should not have accepted"));
}
}
};
Expand Down Expand Up @@ -250,7 +186,7 @@ public Response.Listener getResponseListener()
@Override
public void onSuccess(Response response)
{
org.eclipse.jetty.client.api.Request request = response.getRequest();
var request = response.getRequest();
HttpConversation conversation = ((HttpRequest)request).getConversation();
// Reset the conversation listeners, since we are going to receive another response code
conversation.updateResponseListeners(null);
Expand All @@ -259,14 +195,13 @@ public void onSuccess(Response response)
if (exchange != null && response.getStatus() == HttpStatus.EARLY_HINT_103)
{
// All good, continue.
System.err.println("onSuccess\n" + response.getHeaders());
hints.add(response.getHeaders().get("Hint"));
exchange.resetResponse();
exchange.proceed(null);
}
else
{
throw new IllegalStateException("should not have accepted");
response.abort(new IllegalStateException("should not have accepted"));
}
}
};
Expand Down

0 comments on commit bfa8c65

Please sign in to comment.