page.js icon indicating copy to clipboard operation
page.js copied to clipboard

Paths URI-decoded twice

Open gertsonderby opened this issue 10 years ago • 3 comments

Within Route.match(), there is the following construction:

m = this.regexp.exec(decodeURIComponent(pathname));
//...     
for (var i = 1, len = m.length; i < len; ++i) {
  //...
  var val = decodeURLEncodedURIComponent(m[i]);
  //...
}

As a result, unless the option decodeURLComponents is passed to Page.js, the path is first URI-decoded, and then the individual components are decoded again. As a result, a path which contains a URI-encoded % blows up with an "URIError: URI malformed" message. If the option is passed, the path is decoded, but only before matching, which means that a URI-encoded / confounds the matching.

At a guess, the first decoding is undesirable?

gertsonderby avatar Aug 14 '15 14:08 gertsonderby

How about changing decodeURLEncodedURIComponent -> decodeURIComponent in for loop?

I may misunderstand but I found that this changes + sign in url to %20 (empty space) unnecessary. (Unnecessary twice call decodeURLEncodedURIComponent()?)

m = this.regexp.exec(decodeURIComponent(pathname));
//...     
for (var i = 1, len = m.length; i < len; ++i) {
  //...
  var val = decodeURLEncodedURIComponent(m[i]);
  //...
}

%2B ----decodeURIComponent() ---> + ----decodeURLEncodedURIComponent()--->

In Cloudera Hue product, some version used this module and there is a bug in file browser. It malfunctions when opening file of which name has + sign. https://github.com/cloudera/hue/issues/931

eubnara avatar Oct 28 '19 18:10 eubnara

@heymagurany

~~Hello, I think just making decodeURLComponents to false is not proper solution.~~ (https://github.com/visionmedia/page.js/pull/348#issuecomment-552688558)

~~The PR you tried to fix this problem should be merged.~~

Fix: Oh I understand now. logic has slightly changed.

eubnara avatar Apr 27 '20 03:04 eubnara

https://github.com/visionmedia/page.js/blob/master/index.js#L794-L806 Duplicate decoding still exists.

eubnara avatar Apr 27 '20 03:04 eubnara