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

Support fragments in UriComponentsBuilder.fromHttpUrl() #25300

Closed
ljosefik opened this issue Jun 22, 2020 · 6 comments
Closed

Support fragments in UriComponentsBuilder.fromHttpUrl() #25300

ljosefik opened this issue Jun 22, 2020 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@ljosefik
Copy link

ljosefik commented Jun 22, 2020

Method fromHttpUrl ends with IllegalArgumentException if parameter contain fragment

java.lang.IllegalArgumentException: [https://tools.ietf.org/html/rfc6749#section-4.1] is not a valid HTTP URL

Test case:

        String redirectUri = "https://tools.ietf.org/html/rfc6749#section-4.1";
        java.net.URL url1 = new URL(redirectUri);
        UriComponentsBuilder urlBuilder = UriComponentsBuilder.fromUriString(redirectUri);
        //UriComponentsBuilder urlBuilder = UriComponentsBuilder.fromHttpUrl(redirectUri);

From code (for URL '#' missing):

	// Regex patterns that matches URIs. See RFC 3986, appendix B
	private static final Pattern URI_PATTERN = Pattern.compile(
			"^(" + SCHEME_PATTERN + ")?" + "(//(" + USERINFO_PATTERN + "@)?" + HOST_PATTERN + "(:" + PORT_PATTERN +
					")?" + ")?" + PATH_PATTERN + "(\\?" + QUERY_PATTERN + ")?" + "(#" + LAST_PATTERN + ")?");

	private static final Pattern HTTP_URL_PATTERN = Pattern.compile(
			"^" + HTTP_PATTERN + "(//(" + USERINFO_PATTERN + "@)?" + HOST_PATTERN + "(:" + PORT_PATTERN + ")?" + ")?" +
					PATH_PATTERN + "(\\?" + LAST_PATTERN + ")?");
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 22, 2020
@sbrannen
Copy link
Member

I've edited your comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jun 26, 2020
@sbrannen
Copy link
Member

Changing the RegEx pattern as follows would support fragments in the HTTP URL, but I'll hold off on implementing the change until I receive feedback from the team.

private static final Pattern HTTP_URL_PATTERN = Pattern.compile(
		"^" + HTTP_PATTERN + "(//(" + USERINFO_PATTERN + "@)?" + HOST_PATTERN + "(:" + PORT_PATTERN + ")?" + ")?" +
				PATH_PATTERN + "(\\?" + QUERY_PATTERN + ")?" + "(#" + LAST_PATTERN + ")?");

@sbrannen sbrannen changed the title UriComponentsBuilder is not working with valid URL UriComponentsBuilder.fromHttpUrl() does not support fragments Jun 26, 2020
@sbrannen sbrannen self-assigned this Jun 26, 2020
@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 26, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jun 26, 2020

@ljosefik what is the context in which you're using this? As you probably know a fragment is not sent with HTTP requests and is not available on the server side, so it is a client-side concept only, used in browsers.

If you need to parse with the complete URI syntax including fragments, you can use fromHttpUri.

@ljosefik
Copy link
Author

Yes, method fromHttpUri can be used and we solved our problem with this approach.
Context - on backend we validate URL used in configuration.
Generally, character '#' is valid as part of URL, so HTTP_URL_PATTERN should validate as valid also "https://tools.ietf.org/html/rfc6749#section-4.1". This example shows that fragment is not used only on frontend side but also as an reference. Also popular OAuth2 framework used fragment in response from server side , please see https://tools.ietf.org/html/rfc6749#section-4.2.2

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 26, 2020
@rstoyanchev
Copy link
Contributor

Yes fragment is definitely valid syntax. I wasn't making an argument the other way but simply guessing at this point as to the original intent which was quite a long time ago.

I think we can change it to parse fragments if present. It should be lenient on parsing and also the fragment method can be used thereafter which is a little inconsistent with the parsing then.

@rstoyanchev rstoyanchev added this to the 5.3 M2 milestone Jun 26, 2020
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: feedback-provided Feedback has been provided labels Jun 26, 2020
@sbrannen sbrannen modified the milestones: 5.3 M2, 5.2.8 Jun 26, 2020
@sbrannen sbrannen changed the title UriComponentsBuilder.fromHttpUrl() does not support fragments Support fragments in UriComponentsBuilder.fromHttpUrl() Jun 26, 2020
@sbrannen
Copy link
Member

Beginning with Spring Framework 5.2.8, UriComponentsBuilder.fromHttpUrl() will support fragments in URLs (see 9876ca0).

sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jun 26, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this issue Aug 16, 2020
Prior to this commit, UriComponentsBuilder.fromHttpUrl() threw an
IllegalArgumentException if the provided URL contained a fragment.

This commit aligns the implementation of fromHttpUrl() with that of
fromUriString(), by parsing a fragment and storing it in the builder.

Closes spring-projectsgh-25300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants