Hi Greg,
Thanks for the insights!
On the question of whether or not the handler should even be invoked when
there is a timeout and delayDispatchUntilContent = true, I'd look at it
through the lens of the "principle of least surprise"... but unfortunately,
you're probably going to surprise someone either way, as you and Simone are
already aware (based on the very nice github issue discussion).
All I can add is that I personally was surprised by this change in
behavior, and probably would *not* be surprised if already-timed-out
requests made it to the handler, because the fact that they're already
timed out is only known because of the delayDispatchUntilContent
optimization, and they would have timed out inside the handler anyway
without the optimization.
Regarding the workaround (setting delayDispatchUntilContent = false), the
only thing that worries me is that it still doesn't behave the same as
Jetty 9.2.x unless you comment out the three client-side lines that were
commented out in the github issue (Thread.sleep() + last output.write() +
last output.flush()). If those lines are commented out, then the tested
scenario is "a client sends a request without sending the entire body and
then tries to read the response"; but with the original lines uncommented,
the tested scenario is "a client sends a full request, but pauses
temporarily somewhere in the middle long enough to trigger the timeout,
finishes sending the full request, and then reads the response".
The second scenario (where the full request is actually sent to the server)
is the one we were trying to test, and with Jetty 9.4.x and
delayDispatchUntilContent = false you get a SocketException [1] when the
client tries to read the response from the socket input stream. Do you know
why this would be? Is Jetty 9.4 somehow more aggressive about closing
sockets than 9.2 was? The key seems to be that final client-side flush to
the socket output stream: without it, the client is able to read back the
408 response; with it, the client is not able to read anything from the
socket.
Take care,
Daniel
[1] Exception in thread "main" java.net.SocketException: Software caused
connection abort: recv failed
at java.net.SocketInputStream.socketRead0(Native Method)
at java.net.SocketInputStream.socketRead(SocketInputStream.java:116)
at java.net.SocketInputStream.read(SocketInputStream.java:171)
at java.net.SocketInputStream.read(SocketInputStream.java:141)
at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
at java.io.InputStreamReader.read(InputStreamReader.java:184)
at java.io.Reader.read(Reader.java:140)
at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:2537)
at org.apache.commons.io.IOUtils.copyLarge(IOUtils.java:2516)
at org.apache.commons.io.IOUtils.copy(IOUtils.java:2493)
at org.apache.commons.io.IOUtils.copy(IOUtils.java:2441)
at org.apache.commons.io.IOUtils.toString(IOUtils.java:1084)
at JettyTimeoutTest.main(JettyTimeoutTest.java:53)
On Thu, Dec 28, 2017 at 5:47 AM, Greg Wilkins <[email protected]> wrote:
> Daniel,
>
> check the issue. I have an explanation and a work around. Not yet sure
> about the fix or even if it should be fixed.
>
> cheers
>
>
> On 28 December 2017 at 11:34, Greg Wilkins <[email protected]> wrote:
>
>> https://github.com/eclipse/jetty.project/issues/2081
>>
>> On 28 December 2017 at 11:01, Greg Wilkins <[email protected]> wrote:
>>
>>> Actually, never mind I will open the issue.
>>>
>>> stand by...
>>>
>>>
>>> On 28 December 2017 at 10:53, Greg Wilkins <[email protected]> wrote:
>>>
>>>> Daniel,
>>>>
>>>> well that looks wrong! I can confirm your test fails for me!
>>>>
>>>> I find it hard to believe we don't have a test for this... but we have
>>>> changed some close handling recently so obviously we have stuffed up
>>>> somehow. investigating....
>>>>
>>>> Please open this as an issue.
>>>>
>>>> regards
>>>>
>>>>
>>>>
>>>>
>>>> On 28 December 2017 at 01:41, Daniel Gredler <[email protected]>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I'm upgrading dependencies in a JAX-RS project, and have reached the
>>>>> point where I need to upgrade Jersey, which (via transitive dependencies)
>>>>> involves upgrading Jetty from 9.2.x to 9.4.x.
>>>>>
>>>>> One of our regression tests verifies that in the case of a request
>>>>> timeout we are sending an HTTP 408 error back to the client. Ignoring all
>>>>> of the JAX-RS layers, the code basically expects that the request handler
>>>>> is able to catch an IOException with getCause() instanceof
>>>>> TimeoutException
>>>>> (see code below for a minimal test case). This was the behavior in Jetty
>>>>> 9.2.x.
>>>>>
>>>>> This appears to have changed in Jetty 9.3.0 and all subsequent 9.3.x
>>>>> and 9.4.x releases: no IOException is thrown in the request handler when
>>>>> the request timeout expires. Connections and so on seem to be cleaned up
>>>>> on
>>>>> the server side correctly, but we no longer have a hook to send the 408
>>>>> response back to the client.
>>>>>
>>>>> What is the best practice in Jetty 9.4.x for hooking into the request
>>>>> timeout mechanism in order to customize the response that is sent back to
>>>>> the client in case of timeouts?
>>>>>
>>>>> Thanks!!
>>>>>
>>>>> Daniel
>>>>>
>>>>>
>>>>>
>>>>> [code sample]
>>>>>
>>>>> import static java.nio.charset.StandardCharsets.UTF_8;
>>>>>
>>>>> import java.io.IOException;
>>>>> import java.io.OutputStreamWriter;
>>>>> import java.net.InetAddress;
>>>>> import java.net.Socket;
>>>>> import java.util.concurrent.TimeoutException;
>>>>>
>>>>> import javax.servlet.http.HttpServletRequest;
>>>>> import javax.servlet.http.HttpServletResponse;
>>>>>
>>>>> import org.apache.commons.io.IOUtils;
>>>>> import org.eclipse.jetty.server.Request;
>>>>> import org.eclipse.jetty.server.Server;
>>>>> import org.eclipse.jetty.server.handler.AbstractHandler;
>>>>>
>>>>> public class JettyTimeoutTest {
>>>>>
>>>>> public static void main(String... args) throws Exception {
>>>>>
>>>>> int port = 8181;
>>>>> Server jetty = new Server(port);
>>>>> jetty.setHandler(new TestHandler());
>>>>> jetty.start();
>>>>>
>>>>> try (Socket s = new Socket(InetAddress.getByName("localhost"),
>>>>> port)) {
>>>>>
>>>>> String body = "abcdefghijklmnopqrstuvwxyz";
>>>>>
>>>>> OutputStreamWriter sw = new
>>>>> OutputStreamWriter(s.getOutputStream(),
>>>>> UTF_8);
>>>>> sw.write("POST /foo HTTP/1.0\r\n");
>>>>> sw.write("Content-Length: " + body.length() + "\r\n");
>>>>> sw.write("Content-Type: application/x-www-form-urlenco
>>>>> ded\r\n");
>>>>> sw.write("\r\n");
>>>>> sw.flush();
>>>>> sw.write(body.substring(0, 10));
>>>>> Thread.sleep(40_000); // a little longer than the server
>>>>> timeout, which is 30 seconds
>>>>> sw.write(body.substring(10));
>>>>> sw.flush();
>>>>>
>>>>> String response = IOUtils.toString(s.getInputStream(),
>>>>> UTF_8);
>>>>> System.out.println("Response:");
>>>>> System.out.println(response); // 408 response in Jetty <=
>>>>> 9.2.23, empty response in Jetty >= 9.3.0
>>>>>
>>>>> } finally {
>>>>> jetty.stop();
>>>>> }
>>>>> }
>>>>>
>>>>> private static class TestHandler extends AbstractHandler {
>>>>>
>>>>> @Override
>>>>> public void handle(String target, Request baseRequest,
>>>>> HttpServletRequest request, HttpServletResponse response)
>>>>> throws IOException {
>>>>>
>>>>> int status;
>>>>> String body;
>>>>>
>>>>> try {
>>>>> status = HttpServletResponse.SC_OK;
>>>>> body = IOUtils.toString(request.getInputStream(),
>>>>> UTF_8); // tries to read the entire POST body
>>>>> } catch (IOException e) {
>>>>> if (e.getCause() instanceof TimeoutException) {
>>>>> status = HttpServletResponse.SC_REQUEST_TIMEOUT;
>>>>> body = "HTTP ERROR 408";
>>>>> } else {
>>>>> status = HttpServletResponse.SC_INTERNA
>>>>> L_SERVER_ERROR;
>>>>> body = "HTTP ERROR 500";
>>>>> }
>>>>> }
>>>>>
>>>>> response.setContentType("text/plain");
>>>>> response.setStatus(status);
>>>>> response.getWriter().write(body);
>>>>> baseRequest.setHandled(true);
>>>>> }
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> jetty-users mailing list
>>>>> [email protected]
>>>>> To change your delivery options, retrieve your password, or
>>>>> unsubscribe from this list, visit
>>>>> https://dev.eclipse.org/mailman/listinfo/jetty-users
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Greg Wilkins <[email protected]> CTO http://webtide.com
>>>>
>>>
>>>
>>>
>>> --
>>> Greg Wilkins <[email protected]> CTO http://webtide.com
>>>
>>
>>
>>
>> --
>> Greg Wilkins <[email protected]> CTO http://webtide.com
>>
>
>
>
> --
> Greg Wilkins <[email protected]> CTO http://webtide.com
>
> _______________________________________________
> jetty-users mailing list
> [email protected]
> To change your delivery options, retrieve your password, or unsubscribe
> from this list, visit
> https://dev.eclipse.org/mailman/listinfo/jetty-users
>
_______________________________________________
jetty-users mailing list
[email protected]
To change your delivery options, retrieve your password, or unsubscribe from
this list, visit
https://dev.eclipse.org/mailman/listinfo/jetty-users
