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
Assignees

Comments

@bjorncs
Copy link
Contributor

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 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 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 <[email protected]>
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 <[email protected]>
@sbordet
Copy link
Contributor

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 <[email protected]>
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 <[email protected]>
(cherry picked from commit aed20ab)
@joakime
Copy link
Contributor

joakime commented Apr 8, 2021

Closing, as PR #6108 was merged.

@joakime joakime closed this as completed 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 <[email protected]>
(cherry picked from commit aed20ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants
@joakime @sbordet @bjorncs and others