Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Introduce UriCompliance.Violation.FRAGMENT to reject HTTP Request Line that includes fragment section. #11579

Closed
joakime opened this issue Mar 27, 2024 · 4 comments · Fixed by #12504
Closed

Introduce UriCompliance.Violation.FRAGMENT to reject HTTP Request Line that includes fragment section. #11579

joakime opened this issue Mar 27, 2024 · 4 comments · Fixed by #12504
Assignees
@joakime
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@joakime
Copy link
Contributor

joakime commented Mar 27, 2024 •
edited
Loading

Jetty version(s)
12.0.7

Jetty Environment
All

Java version/vendor (use: java -version)
All

OS type/version
All

Description
While working PR #11496 the idea of not allowing FRAGMENT section in a Request Line was introduced.

It is good idea that seems to follow the HTTP spec.

If we do this, we should be careful how we do it, and allow a configurable UriCompliance mode to configure the behavior.

See original commit (reverted in PR #11496):
fed10f7
@joakime joakime added Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc) labels Mar 27, 2024
joakime added a commit that referenced this issue Mar 27, 2024
@joakime
Issue #11579 - Reject Query in HttpURI
3f06eb3
@joakime joakime self-assigned this Mar 27, 2024
@joakime joakime moved this to 🏗 In progress in Jetty 12.0.9 - FROZEN Mar 27, 2024
@joakime joakime linked a pull request Mar 27, 2024 that will close this issue
Issue #11579 - Reject Target in Request Path #11580
Closed
@joakime
Copy link
Contributor Author

joakime commented Mar 27, 2024

Opened PR #11580 to start this issue.
Currently just a cherry-pick of commit fed10f7 along with some testcase updates

@sbordet
Copy link
Contributor

sbordet commented Mar 27, 2024

@joakime can you please reword everywhere you wrote "query" meaning "fragment"?

@joakime joakime changed the title Introduce UriCompliance.Violation.FRAGMENT to reject HTTP Request Line that includes Query section. Introduce UriCompliance.Violation.FRAGMENT to reject HTTP Request Line that includes fragment section. Mar 27, 2024
@joakime
Copy link
Contributor Author

joakime commented Mar 27, 2024

Yeah, sorry, my mind was stuck on the exception message from the old PR ...

jetty.project/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java

Lines 1460 to 1461 in fed10f7

if (last != null && state.ordinal() > last.ordinal())
throw new IllegalArgumentException("uri cannot go beyond " + last);

The exception messages from that commit showed up as ...

HTTP/1.1 400 Bad Request
Server: Jetty(12.0.8-SNAPSHOT)
Date: Wed, 27 Mar 2024 08:30:11 GMT
Cache-Control: must-revalidate,no-cache,no-store
Content-Type: text/html;charset=iso-8859-1
Content-Length: 621
<html>
<head>
<meta http-equiv="Content-Type" content="text/html;charset=ISO-8859-1"/>
<title>Error 400 Bad Request</title>
</head>
<body>
<h2>HTTP ERROR 400 Bad Request</h2>
<table>
<tr><th>URI:</th><td>/badMessage</td></tr>
<tr><th>STATUS:</th><td>400</td></tr>
<tr><th>MESSAGE:</th><td>Bad Request</td></tr>
<tr><th>CAUSED BY:</th><td>org.eclipse.jetty.http.BadMessageException: 400: Bad Request</td></tr>
<tr><th>CAUSED BY:</th><td>java.lang.IllegalArgumentException: uri cannot go beyond QUERY</td></tr>
</table>
<hr/><a href="https://eclipse.org/jetty">Powered by Jetty:// 12.0.8-SNAPSHOT</a><hr/>
</body>
</html>

@joakime joakime moved this to 🏗 In progress in Jetty 12.0.10 (FROZEN) Apr 26, 2024
@joakime joakime moved this to 🏗 In progress in 🧊 Jetty 12.0.12 - FROZEN Jun 25, 2024
@joakime joakime moved this to 🏗 In progress in Jetty 12.0.13 - FROZEN Jul 24, 2024
@gregw gregw moved this to 👀 In review in Jetty 12.0.14 Aug 28, 2024
@joakime joakime removed this from Jetty 12.0.14 Sep 24, 2024
@gregw
Copy link
Contributor

gregw commented Nov 8, 2024

https://www.rfc-editor.org/rfc/rfc9110.html#section-7.1 says ignore/drop rather than reject?!? So perhaps we do not include this violation by default?

gregw added a commit that referenced this issue Nov 8, 2024
@gregw
Fix #11579 Added FRAGMENT UriCompliance Violation
8f709bb
@gregw gregw mentioned this issue Nov 8, 2024
Merged
@gregw gregw linked a pull request Nov 8, 2024 that will close this issue
UriCompliance Violation FRAGMENT #12504
Merged
gregw added a commit that referenced this issue Nov 8, 2024
@gregw
Fix #11579 Added FRAGMENT UriCompliance Violation
99e816a
gregw added a commit that referenced this issue Nov 11, 2024
@gregw
UriCompliance Violation FRAGMENT (#12504)
b5a99e7 
* Fix #11579 Added FRAGMENT UriCompliance Violation

* Fix #11579 Added FRAGMENT UriCompliance Violation
@joakime joakime closed this as completed Jan 15, 2025
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.1.0 Jan 15, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@joakime joakime

Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)
Projects
Jetty 12.1.0
Status: ✅ Done
Milestone
No milestone
3 participants
@gregw @joakime @sbordet