spring:url tag does not correctly encode forward slash / in path variable. Should be calling encodePathSegment [SPR-11401]

[spring-projects-issues]

daniel carter opened SPR-11401 and commented

The UrlTag advises that it will URL encode template URI variables

I have a path URL in my controller

@RequestMapping(value = "/circuit/{id}/view", method = RequestMethod.GET)

And the following in my jsp to call it:

<spring:url var="viewUrl" value="/circuit/{circuitId}/view" >
    <spring:param name="circuitId"  value="my/Id"/>
  </spring:url>

If for example id=my/Id then the tag writes out /circuit/my/Id/view

This obviously does not work as it fails to match the request mapping.

It should be URL encoding the path variable to write out /circuit/my%2FId/view

The problem is that / has a special meaning in paths, and thus needs to be escaped in path variables, but UrlTag.replaceUriTemplateParams calls UriUtils.encodePath(...) and ultimately at line 480 of HierarchicalUriComponents it decides that '/' does not need encoding

PATH {
		@Override
		public boolean isAllowed(int c) {
			return isPchar(c) || '/' == c;
		}
	},

Looks like UrlTag should be calling UriUtils.encodePathSegment(...) instead.

---

Affects: 3.2.5

Issue Links:

• Spring tag library does not encode a URI correctly [SPR-10303] #14937 Spring tag library does not encode a URI correctly ("is duplicated by")
• Support advanced URI Template Syntax [SPR-10505] #15137 Support advanced URI Template Syntax
• Spring request mapping annotation does not map an encoded URI correctly [SPR-10306] #14940 Spring request mapping annotation does not map an encoded URI correctly

[spring-projects-issues]

Rossen Stoyanchev commented

The challenge is that there is no way of indicating whether you want to expand the URI variable as a path or as a path segment. Obviously in your case you want it expanded as a path segment but it is also legitimate in other cases to want to expand it as a path (i.e. with the "/" preserved). The answer may be in using allowing some more advanced URI template syntax (see #15137). In the mean time we can't really change this behavior without causing regressions.

[spring-projects-issues]

daniel carter commented

How about a parameter to spring:url, or the individual spring:param to control the encoding that is applied during substitution?

I'm surprised people would want to expand the variable as a path hierarchy (preserving the /), rather than as PathVariable (and thus encoding the /). Is that really going to be common compared to using it as a PathVariable?

In the meantime i will have to encode the variable myself before passing to spring:url. Nasty and makes traditional URL parameters much more attractive than PathVariables. I.e. /circuit/view?id=my%2FAId

[spring-projects-issues]

daniel carter commented

Suggest the naming of any-such parameter be closely related to any parameter used in the solutions to #15727

[spring-projects-issues]

daniel carter commented

The more i think about this, the more i think that not escaping the / by default is broken.

Now i'm fairly new to this path variable style of URI, so i could be way off the ball, but...

It seems to me that the concept of a path variable is to take one segment of a path and treat it as a variable, where path segments are the parts of the URL after the hostname delimiated by /.

As / is the delimiter for path segments, it follows that path variables cannot contain the / without first escaping it. Much like in a csv file, a row entry cannot contain a comma without first escaping it. If a csv library wrote out a row entry without escaping commas it would be considered a bug, as it will cause parsing of the file to fail. Likewise when the spring:url tag writes out a path variable it needs to escape any /, otherwise it would be writing out two path segments rather than one, and the server processing a request to any such URL will fail to parse it.

What are the valid use-cases for a single path variable mapping to two path segments? How would parsing this ever work on the server side? Wouldn't you rather need to have /path/to/{month}/{day} rather than /path/to/{monthday} where monthday=month+"/"+day, in order for the server to make any sense of it?

i.e. if you did allow when writing out the URL for a variable {monthday} to contain 10/02 and wrote that out literally without escaping it, on the server side it wouldn't match the same url pattern /path/to/{monthday}. You would need on the server side to use /path/to/{month}/{day}

[spring-projects-issues]

daniel carter commented

Seem RFC 6570 in #15137 is thinking along the same lines.

3.2.6. Path Segment Expansion: {/var}

Path segment expansion, as indicated by the slash ("/") operator in
Level 3 and above templates, is useful for describing URI path
hierarchies.

For each defined variable in the variable-list, append "/" to the
result string and then perform variable expansion, as defined in
Section 3.2.1, with the allowed characters being those in the
unreserved set.

Note that the expansion process for path segment expansion is
identical to that of label expansion aside from the substitution of
"/" instead of ".". However, unlike ".", a "/" is a reserved
character and will be pct-encoded if found in a value.

[spring-projects-issues]

daniel carter commented

This is quite analogous to old style query parameters

If instead of /circuit/{id} i had /circuits?id={id} and my id contained a &, the url tag (and the rest of the spring stuff) would just encode it no problems as it's a reserved character. Nobody asks, but what if you intended for the value of the id parameter to expand out into a raw query string.
e.g. you set
id="abc&view=minimal&c=5"
to create the URL
/circuits?id=abc&view=minimal&c=5
would just be crazy. The expected behaviour for id="abc&def" is
/circuits?id=abc%26def

[spring-projects-issues]

Rossen Stoyanchev commented

I've added support for this. See the description for this commit.