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

Special characters in path variables are escaped multiple times when added with slash() #40

Closed
pgpx opened this issue Jan 2, 2013 · 9 comments

Comments

@pgpx
Copy link

pgpx commented Jan 2, 2013

I'm trying to use a URI-encoded value as a path segment value when constructing a link with LinkBuilder's slash() method, but I find that any 'special' characters (such as %) will be repeatedly double-encoded by each subsequent slash() call.

For example:

linkToCurrentMapping().slash("first")
    .slash("sec%ond").slash("thi%rd")
    .slash("fourth").withSelfRel();

Should return http://localhost/first/sec%25ond/thi%25rd/fourth but gets http://localhost/first/sec%252525ond/thi%2525rd/fourth

I think that this is being caused by LinkBuilderSupport's interaction with HierarchicalUriComponentBuilder.toUri and URI (which always encodes the path).

Note that I'm actually trying to percent-encode a path as a path segment value, but if I don't explicitly percent-encode slashes in the path, then they are not encoded at all:

linkToCurrentMapping().slash("first")
    .slash("sec/ond").slash("third").withSelfRel();

I would like http://localhost/first/sec%2Fond/third/fourth but get http://localhost/first/sec/ond/third (due to the StringUtils.tokenizeToStringArray(object.toString(), "/") call in LinkBuilderSupport.slash).

Here's the full unit test: https://gist.github.com/4436408

@drewcox
Copy link

drewcox commented Dec 11, 2013

I too think I am seeing this behaviour, when trying to include a "{" in a path segment via a path parameter in the MVC Controller method mapping. The resulting encoding is "%257B" instead of the expected "%7B", due to double encoding.

I did a little picking through the source, and basically is seems the parameter is encoded once when going through the "expand" process for the URL template generated from controller method, then the entire path value is re-encoded when adding the link to the resource. Rough description...

@felixbarny
Copy link

I have a simmilar issue when using linkTo(methodOn(...)) if there are e.g. whitespaces in the parameters.
I think the problem sits in the toUri Method:

public URI toUri() {
    return uriComponents.encode().toUri();
}

imho it should be:

public URI toUri() {
    return uriComponents.toUri();
}

@felixbarny
Copy link

If someone faces the same issue, here is my workaround:

public final class LinkBuilderUtil {
    // ControllerLinkBuilder.linkTo has a problem: it double-URL-encodes characters!
    // e.g. ' ' (whitespace) is encoded to '%2525' instead of '%20'
    // TODO remove hack when https://github.com/spring-projects/spring-hateoas/issues/40 is resolved
    private static final Field uriComponentsField;

    static {
        try {
            uriComponentsField = LinkBuilderSupport.class.getDeclaredField("uriComponents");
            uriComponentsField.setAccessible(true);
        } catch (NoSuchFieldException e) {
            throw new RuntimeException(e);
        }
    }

    private LinkBuilderUtil() {
    }

    public static String relativeLinkTo(Object invocationValue) {
        return UriComponentsBuilder.fromUriString(linkTo(invocationValue)).scheme(null).host(null).build().toUriString();
    }

    public static String linkTo(Object invocationValue) {
        try {
            final UriComponents uriComponents = (UriComponents) uriComponentsField.get(ControllerLinkBuilder.linkTo(invocationValue));
            return uriComponents.toString();
        } catch (IllegalAccessException e) {
            throw new RuntimeException(e);
        }
    }
}

@meyertee
Copy link

Seems related to #96 which is marked as fixed, but it doesn't seem to be, I'm having the same issue using the LinkBuilder in various ways:

Link link = ControllerLinkBuilder.linkTo(ThingController.class).slash("my%20thing").withSelfRel();
link.getHref(); // returns "http://localhost:8080/things/my%2520thing"

Same happens using the EntityLinks:

Link link = entityLinks.linkToSingleResource(ThingResource.class, "my%20thing");

@acorcoran-daon
Copy link

It appears that the problem is in UriTemplate:

public URI expand(Object... uriVariableValues) {
    UriComponents expandedComponents = this.uriComponents.expand(uriVariableValues);
    UriComponents encodedComponents = expandedComponents.encode();
    return encodedComponents.toUri();
}

This expandedComponents.encode() is causing any arguments for the URI template to be encoded. When the URI is encoded once again later ....

@odrotbohm
Copy link
Member

We've just pushed a removals for an unnecessary back-and-forth conversion in the course of the fix for #398. Does that maybe even fix that issue here?

@odrotbohm odrotbohm self-assigned this Jan 12, 2016
@pgpx
Copy link
Author

pgpx commented Jan 12, 2016

This fixes my first test (thanks!), so the following passes:

Link l = linkToCurrentMapping().slash("first")
   .slash("sec%ond").slash("thi%rd").slash("fourth").withSelfRel();
assertEquals("http://localhost/first/sec%25ond/thi%25rd/fourth", l.getHref());

However, it doesn't encode any / characters at all - though maybe that is by design?

linkToCurrentMapping().slash("first")
    .slash("sec/ond").slash("third").withSelfRel();

I would like http://localhost/first/sec%2Fond/third but get http://localhost/first/sec/ond/third

@gregturn
Copy link
Contributor

.slash() basically appends "/" with the contents. However, "/" is still the token splitting thing, so .slash("sec/ond") is just like typing slash("sec").slash("ond").

To support this, we'd need to pre-encode the slash() command, which I'm not convinced on.

@gregturn
Copy link
Contributor

I think it's safe to say that if you NEED to encode a "/" and NOT have it split into two paths, you are responsible for that.

There is little way in framework code to detect this rare corner case.

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

No branches or pull requests

7 participants