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

UNDERTOW-2017: HttpRequestConduit state is correctly maintained #1288

Merged
merged 1 commit into from Feb 6, 2022

Conversation

carterkozak
Copy link
Contributor

Previously, small buffers that required multiple events to write
would not save the correct header state, resulting in NPEs
and requests which failed to include all headers.

This PR removes the duplicated state between the method body
and HttpRequestConduit in favor of exclusively using
HttpRequestConduit fields. This way the behavior across state
changes is unlikely to be impacted at all.
Hotspot isn't able to optimize field access quite as well
as local method scoped variables, however this isn't likely
to have an impact in practice, and it's more important to
send all headers with every request.

Previously, small buffers that required multiple events to write
would not save the correct header state, resulting in NPEs
and requests which failed to include all headers.

This PR removes the duplicated state between the method body
and HttpRequestConduit in favor of exclusively using
HttpRequestConduit fields. This way the behavior across state
changes is unlikely to be impacted at all.
Hotspot isn't able to optimize field access quite as well
as local method scoped variables, however this isn't likely
to have an impact in practice, and it's more important to
send all headers with every request.
@carterkozak
Copy link
Contributor Author

Tested with #1287

@fl4via fl4via added bug fix Contains bug fix(es) next release This PR will be merged before next release or has already been merged (for payload double check) labels Jan 20, 2022
@fl4via fl4via merged commit 19a17fe into undertow-io:master Feb 6, 2022
@fl4via fl4via removed the next release This PR will be merged before next release or has already been merged (for payload double check) label Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Contains bug fix(es)
Projects
None yet
2 participants