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

URL parameters with ampersand (&) in value get truncated #212

Open
rocketmonkeys opened this issue Sep 14, 2015 · 2 comments
Open

URL parameters with ampersand (&) in value get truncated #212

rocketmonkeys opened this issue Sep 14, 2015 · 2 comments

Comments

@rocketmonkeys
Copy link
Contributor

Bug

I'm running into this bug right now with pagerjs 1.0.1. I use a page-href like this:

<a data-bind="page-href: { path: '/somepage', params: { param1: 'first &amp; second', param2: 'other value' } }">link</a>

On the page, we have this:

param1 == "first"
param2 == "other value"

Note that param1 is truncated to where the ampersand is in the value.

Cause

The problem appears to be here

var parseHash = function (hash) {
        return $.map(hash.replace(/\+/g, ' ').split('/'), decodeURIComponent);
};

That turns this URL:

/somepage?param1=first+%26+second&param2=other+value

Into this:

['/somepage?param1=first & second&param2=other value']

The problem is that we're decoding the %26 into & too early. Now instead of "param1" having the value "first & second", we have these params:

{
    "param1": "first ",
    " second": undefined,
    "param2": "other value"
}

Fix

Instead of decoding the entire URL in parseHash, we need to be more selective.

  • In parseHash, split off the hash from the rest of the URL (so we'd have "/somepage" and "param1=first+%26+second&param2=other+value" separately).
  • Decode just the page part ("/somepage") at this point, leave the hash as-is.
  • Later when we call parseStringAsParameters(), we need to split on &, then separately decode each piece.

This will preserve ampersands in values w/o messing up the param splitting.

@mw44118
Copy link

mw44118 commented Oct 12, 2015

I'm facing this bug right now. Is there a workaround I can use now, before you release a fix?

@rocketmonkeys
Copy link
Contributor Author

@mw44118 - my workaround is to double-urlencode all values before setting them in the URL, and decode them when using pagerjs's params. This means that the values are decoded twice; once by pagerjs (and put into the page param observables), then a second time by you in order to get the real value. It's not pretty, and the URLs get a bit longer, but it does work safely (since chars like & that mess up pagerjs are encoded & hidden from pagerjs). Also (probably) fixes any other potential param encoding bugs (like ? and = and who knows what else).

Ideally we'll get a fix in pagerjs. I tried, but it quickly became large (ie. the current code parses way too early in the stack, and we need to pass additional parameters through a lot of functions in order to decode at the right point). It's not too complex, but not something I had time to finish unfortunately :(

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

No branches or pull requests

2 participants