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

Improvements to HttpSender. #12111

Merged
merged 3 commits into from
Aug 2, 2024
Merged

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Jul 30, 2024

  • Changed ContentSender demand from iterate()+IDLE to succeeded()+SCHEDULED. This ensures that there is no re-iteration in case a 100 Continue response arrives. This, in turn, avoids that the demand is performed multiple times, causing ISE to be thrown.
  • Changed the 100 Continue action of the proxy Servlet/Handler, that provides the request content, to be executed by the HttpSender, rather than by the HttpReceiver.

* Changed ContentSender demand from iterate()+IDLE to succeeded()+SCHEDULED.
  This ensures that there is no re-iteration in case a 100 Continue response arrives.
  This, in turn, avoids that the demand is performed multiple times, causing ISE to be thrown.
* Changed the 100 Continue action of the proxy Servlet/Handler, that provides the request content, to be executed by the HttpSender, rather than by the HttpReceiver.

Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet requested review from gregw and lorban July 30, 2024 17:07
@joakime
Copy link
Contributor

joakime commented Aug 1, 2024

Didn't you mention that there are small changes to 100-Continue in RFC9112/RFC9110 that you needed to confirm with jetty-client?
Since I didn't see any test cases here, am I right in assuming that the existing test cases are covering the RFC9112/RFC9110 changes adequately?

@sbordet
Copy link
Contributor Author

sbordet commented Aug 1, 2024

@joakime other changes related to handling of 100 Continue are in #12113.

firoj0

This comment was marked as spam.

gregw
gregw previously approved these changes Aug 2, 2024
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I reviewed these changes in #12113 so same javadoc niggles from there applies here. Other than that LGTM

Signed-off-by: Simone Bordet <[email protected]>
@sbordet sbordet merged commit fa143fa into jetty-12.0.x Aug 2, 2024
2 of 4 checks passed
@sbordet sbordet deleted the fix/jetty-12.0.x/httpsender-fixes branch August 2, 2024 09:28
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
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants