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 queries in opaque URLs #32920

Closed
artembilan opened this issue May 29, 2024 · 2 comments
Closed

Support queries in opaque URLs #32920

artembilan opened this issue May 29, 2024 · 2 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@artembilan
Copy link
Member

This is a regression in Spring Framework 6.2:

	@Test
	public void mailToUrlIsExpandedProperly() {
		DefaultUriBuilderFactory uriFactory = new DefaultUriBuilderFactory();
		String mailToUri = "mailto:{to}?subject={subject}";
		String to = "user@example.com";
		String subject = "Test subject";
		URI uri = uriFactory.expand(mailToUri, Map.of("to", to, "subject", subject));
		assertThat(uri.toString()).isEqualTo("mailto:%s?subject=%s".formatted(to, subject));
	}

The functionality comes from Spring WS with Mail and JMS transports: https://docs.spring.io/spring-ws/docs/current/reference/html/#_jms_transport_2.
So, same fails for ulrs like jms:{destination}?deliveryMode={deliveryMode}&priority={priority}.

@poutsma poutsma self-assigned this May 29, 2024
@poutsma poutsma added this to the 6.2.0-M4 milestone May 29, 2024
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression labels May 29, 2024
@quaff
Copy link
Contributor

quaff commented May 30, 2024

FYI, the test is failed with 6.1.8 too, because subject contains spaces.

java.lang.IllegalArgumentException: Illegal character in opaque part at index 36: mailto:user@example.com?subject=Test subject
	at java.base/java.net.URI.create(URI.java:906)
	at org.springframework.web.util.DefaultUriBuilderFactory$DefaultUriBuilder.createUri(DefaultUriBuilderFactory.java:436)
	at org.springframework.web.util.DefaultUriBuilderFactory$DefaultUriBuilder.build(DefaultUriBuilderFactory.java:417)
	at org.springframework.web.util.DefaultUriBuilderFactory.expand(DefaultUriBuilderFactory.java:174)
	at com.example.demo.UriBuilderFactoryTest.mailToUrlIsExpandedProperly(UriBuilderFactoryTest.java:19)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.net.URISyntaxException: Illegal character in opaque part at index 36: mailto:user@example.com?subject=Test subject
	at java.base/java.net.URI$Parser.fail(URI.java:2976)
	at java.base/java.net.URI$Parser.checkChars(URI.java:3147)
	at java.base/java.net.URI$Parser.parse(URI.java:3183)
	at java.base/java.net.URI.<init>(URI.java:623)
	at java.base/java.net.URI.create(URI.java:904)
	... 7 more


@artembilan
Copy link
Member Author

Yeah... I see your point. Missed the part that non-characters have to be encoded.
When I changed the test to this, it passes:

	@Test
	public void mailToUrlIsExpandedProperly() {
		DefaultUriBuilderFactory uriFactory = new DefaultUriBuilderFactory();
		uriFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY);
		String mailToUri = "mailto:{to}?subject={subject}";
		URI uri = uriFactory.expand(mailToUri, Map.of("to", "user@example.com", "subject", "Test subject"));
		assertThat(uri.toString()).isEqualTo("mailto:user%40example.com?subject=Test%20subject");
	}

However now is the question about a default encoding mode which is:

		/**
		 * Pre-encode the URI template first, then strictly encode URI variables
		 * when expanded, with the following rules:
		 * <ul>
		 * <li>For the URI template replace <em>only</em> non-ASCII and illegal
		 * (within a given URI component type) characters with escaped octets.
		 * <li>For URI variables do the same and also replace characters with
		 * reserved meaning.
		 * </ul>
		 * <p>For most cases, this mode is most likely to give the expected
		 * result because in treats URI variables as opaque data to be fully
		 * encoded, while {@link #URI_COMPONENT} by comparison is useful only
		 * if intentionally expanding URI variables with reserved characters.
		 * @since 5.0.8
		 * @see UriComponentsBuilder#encode()
		 */
		TEMPLATE_AND_VALUES,

Doesn't look like it is taken into account when values are set in the build():

			if (encodingMode.equals(EncodingMode.VALUES_ONLY)) {
				uriVars = UriUtils.encodeUriVariables(uriVars);
			}

Probably separate issue as a back-port to 6.1.x.

Thank you for taking a look!

@poutsma poutsma changed the title The new URL parser skips query part for urls like mailto: Support queries in opaque URLs Jun 7, 2024
@poutsma poutsma closed this as completed in 35e8f1c Jun 7, 2024
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: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants