Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpConnection.getBytesIn() incorrect for requests with chunked content #6105

Closed
bjorncs opened this issue Mar 25, 2021 · 4 comments · Fixed by #6108 or #6145
Closed

HttpConnection.getBytesIn() incorrect for requests with chunked content #6105

bjorncs opened this issue Mar 25, 2021 · 4 comments · Fixed by #6108 or #6145

Comments

@bjorncs
Copy link
Contributor

@bjorncs bjorncs commented Mar 25, 2021

Jetty version
9.4.38.v20210224

Java version
11.0.7+10

OS type/version
MacOS 10.15.7

Description
HttpConnection.bytesIn is only aggregated when fillRequestBuffer() is called from onFillable(), not from fillAndParseForContent(). The result is that HttpConnection.getBytesIn() reports a value lower than than the actual number of bytes filled. I have been able to reproduce this for POST requests using chunked encoding.

@sbordet
Copy link
Contributor

@sbordet sbordet commented Mar 25, 2021

@bjorncs thanks for the report, we'll work on the Jetty issue.
How urgent is this for you?

@bjorncs
Copy link
Contributor Author

@bjorncs bjorncs commented Mar 26, 2021

It's not a blocker - we only use the value for logging purposes at the moment.

@sbordet sbordet self-assigned this Mar 26, 2021
sbordet added a commit that referenced this issue Mar 26, 2021
… chunked content

Moved recording of bytes to fillRequestBuffer(),
so they are accounted also for async reads.
Added test case.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet added this to To do in Jetty 9.4.40 via automation Mar 26, 2021
sbordet added a commit that referenced this issue Mar 26, 2021
… chunked content

Fixed test that was too strictly comparing HttpConnection.bytesIn,
that now report a correct, but larger value.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet
Copy link
Contributor

@sbordet sbordet commented Mar 26, 2021

@bjorncs can you try the code in PR #6108?

sbordet added a commit that referenced this issue Apr 8, 2021
#6108)

* Fixes #6105 - HttpConnection.getBytesIn() incorrect for requests with chunked content

Moved recording of bytes to fillRequestBuffer(),
so they are accounted also for async reads.
Added test case.
Fixed test that was too strictly comparing HttpConnection.bytesIn,
that now report a correct, but larger value.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Apr 8, 2021
… chunked content

Moved recording of bytes to fillRequestBuffer(),
so they are accounted also for async reads.
Added test case.
Fixed test that was too strictly comparing HttpConnection.bytesIn,
that now report a correct, but larger value.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
(cherry picked from commit aed20ab)
@joakime joakime moved this from To do to Reviewed in Jetty 9.4.40 Apr 8, 2021
@joakime
Copy link
Member

@joakime joakime commented Apr 8, 2021

Closing, as PR #6108 was merged.

@joakime joakime closed this Apr 8, 2021
Jetty 9.4.40 automation moved this from Reviewed to Done Apr 8, 2021
sbordet added a commit that referenced this issue Apr 10, 2021
… chunked content

Moved recording of bytes to fillRequestBuffer(),
so they are accounted also for async reads.
Added test case.
Fixed test that was too strictly comparing HttpConnection.bytesIn,
that now report a correct, but larger value.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
(cherry picked from commit aed20ab)
@sbordet sbordet added this to To do in Jetty 10.0.3/11.0.3 via automation Apr 10, 2021
@sbordet sbordet moved this from To do to Done in Jetty 10.0.3/11.0.3 Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
4 participants
@joakime @sbordet @bjorncs and others