Skip to content

Slashes are double encoded in hash query params #1973

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

Closed
natefaubion opened this issue May 20, 2015 · 6 comments
Closed

Slashes are double encoded in hash query params #1973

natefaubion opened this issue May 20, 2015 · 6 comments

Comments

@natefaubion
Copy link

I have a link that's something like

<a ui-sref="myState({ path: '/foo' })">Go</a>

Which results in the hash-url #/my-state?path=%252Ffoo. The slash being encoded to %2F and the percent being encoded to %25. In 0.2.13 this double-encoding seemed to work fine, as the new state picks it up correctly (as /foo), but in 0.2.14 and 0.2.15 the new state will only get it decoded once, resulting in %2Ffoo.

@natefaubion
Copy link
Author

I've actually found that ui-router is transitioning twice on this link. If I don't have a slash in the query param, it transitions once. But once I add the slash it does two transitions, one immediately after the other. The first transition gets the correct param, and the second is double encoded. This invokes my controller twice as well.

@natefaubion
Copy link
Author

I've also found that if you provide a direct url like #/my-state?path=/foo (not through a ui-sref link), the router automatically navigates to #/my-state?path=%2Ffoo (not double-encoded) which works, but inserts a new history item, which breaks the back button (when you hit back it just keeps redirecting you over and over again).

@ckniffen
Copy link

This is related to #1645. Before 0.2.14 the issue was more cosmetic but with the addition of the $normalize method on types the decode function will no longer be called for params of type string.

I have created a failing test for it.

it("should encode and decode slashes in parameter values", function () {
    var matcher = new UrlMatcher('/:foo');
    expect(matcher.format({ foo: "/" })).toBe('/%252F');
    expect(matcher.format({ foo: "//" })).toBe('/%252F%252F');
    expect(matcher.exec('/%2F').foo).toBe('/');
    expect(matcher.exec('/%2F%2F').foo).toBe('//');
 });

Previously the test did have assertions for decoding.

Solution 1

Have the string type override $normalize to always call decode

string: {
      encode: valToString,
      decode: valFromString,
      $normalize: function(val){ return this.decode(val); },
      is: function(val) { return val == null || !isDefined(val) || typeof val === "string"; },
      pattern: /[^/]*/
}

Solution 2

Update is method to check for \ encoding.

@piupiupiu
Copy link

Unfortunately, it seems it's not fixed.

My configuration

$stateProvider
    .state('root',{
        url: '/{lang:(?:[A-Za-z]{2}/)?}',
        abstract: true,
        template: '<ui-view />',
    })
    .state('root.index', {
        url: '',
        template: 'Index Controller',
        controller: function() {
            console.log('Index Controller');
        }  
    });

It should match: http://example.com/ and http://example.com/es/.

However, the optional back slash is encoded when you go to $state.go('root.index',{lang:'es'});

and url becomes:

http://example.com/es%252F

After some investigation I found the script is not decoded because the back slash is double encoded:

First of all, there is the following code:

var encoded = param.type.encode(value);

This code launches the re-defined encode function which replaces the slashes with their encoded entities. So es/ becomes es%2F.

Then the string is double encoded here: result += encodeURIComponent(encoded);

Therefore it becomes es%252F.

I managed to fix it this way:

function valToString(val) { return val != null ? val.toString().replace(/\//g, "%2F") : val; }

replaced with

function valToString(val) { return val != null ? encodeURIComponent(val.toString()) : val; }

and

result += encodeURIComponent(encoded);

replaced with

result += encoded;

I'm not quite familiar with ui-router so I don't know if this solution is correct but it fixes my case.

@piupiupiu
Copy link

Update:

Unfortunately, my fix is incomplete.
It does not work in case of ui-sref directive.

<a ui-sref="root.index({lang: 'es/'})">test</a> becomes <a href="es%2F">test</a>

Could anybody (@eddiemonge @nateabele @christopherthielen @ksperling @timkindberg @PascalPrecht) take a look at the issue?

It's been a while since this issue was reported but it is still even does not have the bug label assigned. Did you, guys, miss it accidentally?

@eddiemonge
Copy link
Contributor

I am going to close this as a duplicate of #1645 and if fixing that doesn't fix this then I'll reopen it.

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

Successfully merging a pull request may close this issue.

4 participants