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

recognize fails with URL-encoded params #71

Closed
jamesarosen opened this issue Nov 26, 2015 · 7 comments
Closed

recognize fails with URL-encoded params #71

jamesarosen opened this issue Nov 26, 2015 · 7 comments

Comments

@jamesarosen
Copy link
Contributor

Failing test:

test("A dynamic route with encoded params", function() {
  var handler = {};
  var router = new RouteRecognizer();
  router.add([{ path: "/foo/:bar", handler: handler }]);

  // Passes:
  resultsMatch(router.recognize("/foo/bar%20baz"), [{ handler: handler, params: { bar: "bar baz" }, isDynamic: true }]);

  // Fails:
  resultsMatch(router.recognize("/foo/bar%3fbaz"), [{ handler: handler, params: { bar: "bar?baz" }, isDynamic: true }]);

  // Fails:
  resultsMatch(router.recognize("/foo/bar%23baz"), [{ handler: handler, params: { bar: "bar#baz" }, isDynamic: true }]);
});
@jamesarosen
Copy link
Contributor Author

Hmm. The following test passes:

test("A dynamic route with encoded params", function() {
  var handler = {};
  var router = new RouteRecognizer();
  router.add([{ path: "/foo/:bar", handler: handler }]);

  equal(router.recognize("/foo/bar%20baz")[0].params.bar, "bar baz");
  equal(router.recognize("/foo/bar%23baz")[0].params.bar, "bar%23baz");
  equal(router.recognize("/foo/bar%3fbaz")[0].params.bar, "bar%3fbaz");
});

Perhaps this isn't the source of my problem (which is that PUT /foo/bar %3f baz shows up as PUT /foo/bar in Mirage).

@jamesarosen
Copy link
Contributor Author

OK, I don't have a failing test for this, but I do have a failing test for Pretender.

Changing

path = decodeURI(path);

to

path = decodeURIComponent(path);

in recognize fixes that failure on Chrome, but not PhantomJS.

@seanjohnson08
Copy link

I have a related issue:

In the app that I am currently maintaining, we have some external URLs that are coming in that are improperly escaped. These URLs have been out in the wild now for over 5 years, and there's not a whole lot we can do to change them.

Here's an example of a poorly encoded URL: /compose-message/Parab%E9ns%20pelo%20seu%20novo%20cargol

The key takeaway is that "%E9" - that is the result of calling escape('é'). I know that the router is doing the right thing by using the URI family of functions (they are UTF compliant) but would it be at all possible to either handle these poorly escaped URLs or give me some kind of hook into which I can handle them myself?

@jamesplease
Copy link
Contributor

jamesplease commented Apr 14, 2016

@seanjohnson08 I think providing a hook for that makes a lot of sense.

@jamesarosen any updates? Is this still an issue?

@jamesarosen
Copy link
Contributor Author

No updates. Still very much an issue (on 0.1.9). I'd be happy to work on this if there's consensus on the correct solution. I'm wary of doing a bunch of work that gets thrown away.

@seanjohnson08
Copy link

seanjohnson08 commented Apr 14, 2016

in my case, the solution would be as simple as

try {
  path = decodeURIComponent(path);
} catch (e) {
  path = unescape(path);
}

But the right solution may look a bit different. I at least believe that the fact that decodeURI can throw should be dealt with in a more user-friendly way.

@rwjblue
Copy link
Collaborator

rwjblue commented Jul 4, 2016

I believe that this was resolved by #91 (and included in 0.2.0). That change will be pulled into Ember in 2.7.

@rwjblue rwjblue closed this as completed Jul 4, 2016
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

4 participants