pagerjs icon indicating copy to clipboard operation
pagerjs copied to clipboard

URL parameters with ampersand (&) in value get truncated

Open rocketmonkeys opened this issue 10 years ago • 2 comments

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.

rocketmonkeys avatar Sep 14 '15 16:09 rocketmonkeys

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

mw44118 avatar Oct 12 '15 14:10 mw44118

@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 :(

rocketmonkeys avatar Oct 12 '15 15:10 rocketmonkeys