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

UriComponentsBuilder does not encode "+" properly [SPR-16718] #21259

Closed
spring-issuemaster opened this Issue Apr 11, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Apr 11, 2018

Joseph Mason opened SPR-16718 and commented

I noticed this issue after upgrading from Spring 4 to Spring 5. Basically, certain API calls we had been making with no issues before starting failing. Looking into it deeper, the issue is that the UriComponentsBuilder is not functioning the same as it was before. It's not clear to me if this is a bug or if there's a different way that this class needs to be used now.

This test passes when using Spring 4:

    @Test
    public void shouldEncodePlusSignInUrlProperly() throws Exception {
        //given
        UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.newInstance()
                .scheme("https")
                .host("test.com")
                .port(443);

        //when
        uriComponentsBuilder.queryParam("foo", "bar+");

        //then
        assertThat(uriComponentsBuilder.buildAndExpand().encode().toString()).isEqualTo("https://test.com:443?foo=bar%2B");
    }

But when using Spring 5, it will fail. Instead of this:

https://test.com:443?foo=bar%2B

it produces:

https://test.com:443?foo=bar+

Affects: 5.0 GA, 5.0.1, 5.0.2, 5.0.3, 5.0.4, 5.0.5

Issue Links:

  • #19394 UriComponentBuilder doesn't work with encoded HTTP URL having '+'.
  • #20968 [docs] Explain URI template encoding
  • #21577 Support stricter encoding of URI variables in UriComponents ("is superseded by")

0 votes, 6 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Apr 12, 2018

Brian Clozel commented

Hi Joseph Mason

I believe this issue duplicates #20750 - and you can find the relevant information in the comments there, and especially here. The original change can be tracked to #19394.

The reference documentation has been improved in that space with #20968, check out the dedicated section.

Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Apr 12, 2018

4509812390128 commented

Spring 4.3.5 where '+' is encoded to "%2B":

543:		HierarchicalUriCompoments.QUERY_PARAM {
544:			@Override
545:			public boolean isAllowed(int c) {
546:				if ('=' == c || '+' == c || '&' == c) {   // evaluates to TRUE for '+'
547:					return false;                       // returns false causing it to be encoded as %2B
548:				}
549:				else {
550:					return isPchar(c) || '/' == c || '?' == c;
551:				}
552:			}
553:		},

Spring 5.0.5 call stack where '+' is not encoded to "%2B":

558:		HierarchicalUriComponents.QUERY_PARAM {
559:			@Override
560:			public boolean isAllowed(int c) {
561:				if ('=' == c || '&' == c) { // evaluates to FALSE for '+' - unlike in Spring 4.3.5
562:					return false;
563:				}
564:				else {
565:					return isPchar(c) || '/' == c || '?' == c; // returns TRUE
566:				}
567:			}
568:		},

RFC3986 referenced in the JavaDocs of HierarchicalUriComponents located at http://www.ietf.org/rfc/rfc3986.txt mentions on page 12 that "+" is under the sub-delims category of characters in a URI:

      reserved    = gen-delims / sub-delims

      gen-delims  = ":" / "/" / "?" / "#" / "[" / "]" / "@"

      sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="

The exact commit where this behavior was changed from not allowed to allowed can be found at:
f2e293a

The commit references #19394 which also references the RFC3986 stating that '+' is a valid URL query parameter value and should not be encoded.
A helpful link to an older discussion around handling of the '+' taking place is located here: https://jira.spring.io/browse/SPR-5516?focusedCommentId=47792&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-47792

Rossen Stoyanchev provides sample code at https://jira.spring.io/browse/SPR-14828?focusedCommentId=155189&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-155189 but his code is used on a path parameter. With some slight modification we can use it on a query parameter:

@Test
public void test() {
    DefaultUriBuilderFactory factory = new DefaultUriBuilderFactory();
    factory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY);
    URI uri = factory.uriString("http://localhost:8080/path/print?value={path}").build("test+");
    System.out.println(uri); // prints: http://localhost:8080/path/print?value=test%2B
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jul 13, 2018

Rossen Stoyanchev commented

Joseph Mason, 4509812390128, please follow #21577 for a solution to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment