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

HttpURI.toURI() does not copy fragment #7750

Closed
ritzow opened this issue Mar 18, 2022 · 4 comments
Closed

HttpURI.toURI() does not copy fragment #7750

ritzow opened this issue Mar 18, 2022 · 4 comments
Labels
Bug For general bugs on Jetty side Stale For auto-closed stale issues and pull requests

Comments

@ritzow
Copy link

ritzow commented Mar 18, 2022

Jetty version(s)

11.0.7

Java version/vendor (use: java -version)

Oracle OpenJDK 17 (from jdk.java.net)

OS type/version

Windows 10.

Description
The fragment does not get copied into the URI object, the final parameter in the URI constructor is set to null, even though HttpURI can handle fragments. This is very unexpected, and I assume, a bug.

https://github.com/eclipse/jetty.project/blob/cb127793e5d8b5c5730b964392a9a905ba49191d/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java#L242

How to reproduce?

HttpURI.build(request.getHttpURI()).fragment("a_fragment").toURI().toString() will produce a URI with no fragment.

@ritzow ritzow added the Bug For general bugs on Jetty side label Mar 18, 2022
@gregw
Copy link
Contributor

gregw commented Mar 18, 2022

It was/is kind of a deliberate bug. Browsers should never send a fragment to the server and as HttpURI is primarily a class used for our server to handle/manipulate URIs, we left the fragment out so that even if some non-browser client sent one, we would drop it and not copy it. This was done more so that we didn't have to think about any consequences of propagating a fragment in server rather than from any hard knowledge of a specific problem.

So can you tell us more about your use-case and why you need a fragment in HttpURI? If there us a real usage, then perhaps we can support it. If there is not, then we should at least document that we deliberately drop the fragment.

@ritzow
Copy link
Author

ritzow commented Mar 18, 2022

@gregw Good to know. I was just going to use it in a Location header from the server side to have the browser automatically scroll to a specific tag, which appears to be accepted by all browsers and in the HTTP spec. I ended up just using specific HttpURI methods instead of HttpURI or URI toString() because it gives more control over the format anyways, but it would be good to at least have a javadoc comment on that method mentioning that it doesn't copy the fragment.

@joakime
Copy link
Contributor

joakime commented Mar 18, 2022

Just adding information (for others that see this in the future)

The HTTP protocol, be it HTTP/1.1 or HTTP/2 (or HTTP/3), basically says that the resource being requested is defined by the ABNF for path-absolute as defined by RFC3986: Section 3 with an optional query portion. (this definition for how the protocol is used never has a fragment uri-subcomponent)

See:

The scheme-specific URI handling for http and https, defined in RFC7230, also says ..

The target URI excludes the reference's fragment component, if any,
since fragment identifiers are reserved for client-side processing

@github-actions
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 Mar 19, 2023
@ritzow ritzow closed this as completed Mar 19, 2023
@ritzow ritzow closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2023
gregw added a commit that referenced this issue Feb 29, 2024
Fix #11465 and #7750
HttpURI.toURI user and fragment are retained.
Use to URI(String) constructor, as all URI constructors will parse the URI anyway.
gregw added a commit that referenced this issue Feb 29, 2024
Fix #11465 and #7750
HttpURI.toURI user and fragment are retained.
Use to URI(String) constructor, as all URI constructors will parse the URI anyway.
gregw added a commit that referenced this issue Feb 29, 2024
* HttpURI toURI passes all info

Fix #11465 and #7750
HttpURI.toURI user and fragment are retained.
Use to URI(String) constructor, as all URI constructors will parse the URI anyway.

* HttpURI toURI passes all info

Fix #11465 and #7750
HttpURI.toURI user and fragment are retained.
Use to URI(String) constructor, as all URI constructors will parse the URI anyway.
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 Stale For auto-closed stale issues and pull requests
Projects
None yet
Development

No branches or pull requests

3 participants