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

Review 100 continue handling in Jetty 12 #9206

Open
lachlan-roberts opened this issue Jan 25, 2023 · 5 comments
Open

Review 100 continue handling in Jetty 12 #9206

lachlan-roberts opened this issue Jan 25, 2023 · 5 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@lachlan-roberts
Copy link
Contributor

Description
There are some failing tests for HTTP 100 continue handling in org.eclipse.jetty.ee10.test.rfcs.RFC2616BaseTest.

This test currently is testing only a core handler but is in ee10 tests, so either needs to be updated to test ee10 code or moved to some core test module.

The logic for 100 continue seems to be both in ServletChannel, and the HttpStream implementations (do we need both), and some left in the ee9 nested HttpChannel. There is also some code for this in ProxyHandler, ProxyServlet, AsyncMiddleManServlet which could be reviewed as well.

@lachlan-roberts lachlan-roberts added the Bug For general bugs on Jetty side label Jan 25, 2023
@janbartel
Copy link
Contributor

Failing tests in org.eclipse.jetty.ee10.test.rfcs.RFC2616BaseTest are:

  • test82ExpectNormal test times out reading the socket to the server
  • test82ExpectInvalid server sends response with status 0 instead of 417
  • test82UnexpectWithBody test expects a Connection: close header but none is sent

Tests have been @Disabled with comment //TODO https://github.com/eclipse/jetty.project/issues/9206

@sbordet
Copy link
Contributor

sbordet commented Jan 26, 2023

The problem is the HttpTesting class, which IMHO should be totally removed in favor of the other available ways of reliably parse HTTP/1.1, in particular HttpTester.

It's the tests that are broken, as 100-continue is well tested in core and eeN and even in the case of proxying.

Copy link

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Jan 27, 2024
@joakime joakime removed the Stale For auto-closed stale issues and pull requests label Jan 27, 2024
@gregw
Copy link
Contributor

gregw commented May 26, 2024

Let's fix these tests in 12.1.0 and stop kicking the can along the road.

@gregw
Copy link
Contributor

gregw commented Oct 2, 2024

See #12113 ?

@gregw gregw moved this to 🏗 In progress in Jetty 12.1.0 Nov 19, 2024
gregw added a commit that referenced this issue Nov 20, 2024
Fix #9206
The HttpGenerator persistence was not correctly maintained over intermediate responses.
Deprecate tester in EE11
---------

Signed-off-by: Olivier Lamy <[email protected]>
Co-authored-by: Olivier Lamy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
Status: 🏗 In progress
Development

No branches or pull requests

5 participants