Recent ServletServerHttpRequest.getURI() change breaks CORS requests with encoded characters [SPR-14248]

[spring-projects-issues]

Russell Allen opened SPR-14248 and commented

This change: #18449 introduced a defect where request's with an origin header and where the request url contains query parameter character's that are typically encoded, trigger a URI formatting exception.

A plain Spring Web project with a no-op controller will reproduce this issue. You must pass the origin header though, as this triggers the DefaultCorsProcessor to fully execute. The call will work with a plain url and fail with a url with special characters:

works: http://127.0.0.1:8080/demo/test?param=plain
fails: http://127.0.0.1:8080/demo/test?param=^

The root exception:

java.net.URISyntaxException: Illegal character in query at index 39: http://127.0.0.1:8080/demo/test?param={}
	at java.net.URI$Parser.fail(URI.java:2848)
	at java.net.URI$Parser.checkChars(URI.java:3021)
	at java.net.URI$Parser.parseHierarchical(URI.java:3111)
	at java.net.URI$Parser.parse(URI.java:3053)
	at java.net.URI.<init>(URI.java:588)
	at org.springframework.http.server.ServletServerHttpRequest.getURI(ServletServerHttpRequest.java:96)
	at org.springframework.web.util.UriComponentsBuilder.fromHttpRequest(UriComponentsBuilder.java:282)
	at org.springframework.web.util.WebUtils.isSameOrigin(WebUtils.java:814)
	at org.springframework.web.cors.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:71)
	at org.springframework.web.servlet.handler.AbstractHandlerMapping$CorsInterceptor.preHandle(AbstractHandlerMapping.java:503)
	at org.springframework.web.servlet.HandlerExecutionChain.applyPreHandle(HandlerExecutionChain.java:134)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:954)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:893)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:968)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:859)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:844)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)

I tested this with Jetty 9.3.2 and 9.3.8, and it reproduced in both versions.

The exception is generated when the query parameter contains a curly brace or carat, but many other characters seem to work. I assume that is simply an aspect of the URI encoding spec that I'm less familiar with.

---

Affects: 4.2.5

Issue Links:

• DefaultCorsProcessor's origin comparison is restrictive and inefficient [SPR-14080] #18652 DefaultCorsProcessor's origin comparison is restrictive and inefficient ("duplicates")

[spring-projects-issues]

Rossen Stoyanchev commented

... where the request url contains query parameter character's that are typically encoded

Are they encoded? It doesn't fail when they are and I can't figure out how to submit a URL that's not encoded. What is the client sending the URL?

[spring-projects-issues]

Russell Allen commented

I presume they are properly encoded by Jetty's implementation of the the Request.

I used Postman, a free tool for making Rest requests (HTTP requests in general), as the client for my test. This issue also occurs from browsers running an Angular app making ajax Rest calls (cross-origin), as well as a node based server making Rest calls.

The same request works in 4.2.4 and fails in 4.2.5.

[spring-projects-issues]

Rossen Stoyanchev commented

Nope, it's the client that encodes the request. Jetty is only processing it.

This issue is a duplicate of #18652 which was fixed in 4.2.6 and 4.3 RC1. I suspect you marked affects 4.3 RC1 because of the fix version for #18449. However #18652 still came in time for 4.3 RC1. In the current version of the code, WebUtils does not parse the query at all.

[spring-projects-issues]

Russell Allen commented

Agreed, this is a dupe of #18652.

I swapped 4.3 RC1 into my test and the issue is resolved. You are correct... I had added that label based on seeing it in the fix version for #18449.

Thank you!