Provider is not consuming the whole input stream request causing the connection abort by Jetty

[molsza]

In case the chunked encoding is used and client has send the whole json in next to last message then provider is returning the parsed object immediately, without waiting for the rest of the message (which is the end of the message). This may cause that response is generated before the last chunk gets to the server, which is treated by the jetty as an erroneous situation and causes the connection abort.

In most cases response is delivered to the client before the connection is closed, but sometimes it happens before that and client reports:

 java.net.SocketException: Connection reset
	at java.net.SocketInputStream.read(SocketInputStream.java:210)
	at java.net.SocketInputStream.read(SocketInputStream.java:141)
	at java.io.BufferedInputStream.fill(BufferedInputStream.java:246)
	at java.io.BufferedInputStream.read1(BufferedInputStream.java:286)
	at java.io.BufferedInputStream.read(BufferedInputStream.java:345)
	at sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:735)
	at sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:678)	

The details of the issue can be found in the jetty tracker: jetty/jetty.project#3027

[cowtowncoder]

Hmmh. Jackson not reading the whole content, just what it needs, is a feature (as a general note).
But I can see how this can be problematic with (potentially) reusable HTTP connections.

Jackson 2.9 finally introduced feature DeserializationFeature.FAIL_ON_TRAILING_TOKENS, enabling of which would force reading of all content, including end-of-input. So enabling that by default would be one option I think.
Alternative would be to explicitly constructing JsonParser, and implementing above -- it's easy enough to do, just bind, call nextToken().

But one challenge is whether this is safe to do in 2.9.8 patch or not....

[stevenschlansker]

We think we ran into this issue as well - with the default settings, you can (relatively rarely) observe EOFException coming from a much newer Jetty client.

It seems the fix is to enable DeserializationFeature.FAIL_ON_TRAILING_TOKENS, which is not on by default. Then, the mapper consumes the entire input stream up until reading -1, which is required by the servlet spec.

@cowtowncoder , should ProviderBase or at least JacksonJsonProvider either configure this non-default feature themselves, or error / warn if it is not set on the configured ObjectMapper? This is a huge pitfall if you happen to run into it - as far as I can tell, we are required to consume the entire stream up until reading EOF.

[cowtowncoder]

Interesting.
Given issues like that, yes, it sounds like that setting should be enabled by default; and allow disabling with JaxRSFeature?

I'd be happy to help with a PR; otherwise may or may not have time to tackle this soon -- but with some help could make it in 2.15 release. Also needs to go in Jakarta variant of course.

[stevenschlansker]

Thanks @cowtowncoder , please consider:
#170
FasterXML/jackson-jakarta-rs-providers#16

[cowtowncoder]

Fixed by #170 as per @stevenschlansker 's awesome contribution.

[cowtowncoder]

Ok. I think this works for most (common) cases. Although realized that in theory, since this is "fail fast", it does not necessarily read all content -- it will just either decode one token that follows, or throw exception encountering unexpected content.

But I think in most cases there is at most white space after closing end marker; or, perhaps, empty chunk? If so, this will resolve the problem.

[stevenschlansker]

As I understand it, when you read JSON from a JAX-RS resource, it is supposed to be the entire content. So the trailing data should be empty or maybe a newline if you have pretty-printing on. If there's anything else, an exception will be thrown, so the user will have to figure out whether they just turn off this feature or implement their own strategy for handling any trailing data.

[cowtowncoder]

Agreed @stevenschlansker . Was only thinking of the case of low-level handling -- but then again, yes, such cases (where there is more content, non-whitespace) will become error cases so sub-optimal buffer handling is less of an issue. Since there is a problem to fix anyway.