Prevent removing trailing slash in request URL #96
Conversation
| } | ||
|
|
||
| function stripSlashes(path) { | ||
| if (typeof path === 'string') { |
There was a problem hiding this comment.
Instead of checking for string 3 times, you could either type cast the argument to a string or throw an error if the argument it not a string.
There was a problem hiding this comment.
throw an error if the argument it not a string
Which argument? The one to _buildURL?
There was a problem hiding this comment.
No, I'm referring to typeof path === 'string'. You don't need to check the argument type here.
It's safe enough for stripSlashes to either force path argument to be a string or throw an error here if it's not.
There was a problem hiding this comment.
Oh okay, gotcha, I'll fix that up.
Is there a reason not to do the check? Like a performance concern? Or is it just unnecessary? removing the checks makes a bunch of tests fail that were passing before.
|
Other than one comment, it looks good. |
alexlafroscia
force-pushed
the
fix-trailing-slash-issue
branch
2 times, most recently
from
8319202
to
81aa92c
Compare
|
@taras can you review this again? I took care of the issue you mentioned. |
alexlafroscia
changed the title
alexlafroscia
changed the title
|
👍 |
The check was not actually whether or not a URL was absolute, but whether it was "complete". For example, "http://foo.com" should have been recognized, although that is not actually what an "absolute url" usually refers to, which is something like "/foo". The name change just increases the clarify of what is being verifyied.
The old way of building a URL was too brittle and didn't cover a lot of the different cases that could take place, in different combinations of the host, namespace, and segment having or not having slashes included. The new approach implemented here handles the various cases correctly, and ensures that we do not remove a trailing slash if one was provided. Closes ember-cli#70
alexlafroscia
force-pushed
the
fix-trailing-slash-issue
branch
from
81aa92c
to
b46efae
Compare
The old way of building a URL was too brittle and didn't cover a lot of the different cases that could take place, in different combinations of the host, namespace, and segment having or not having slashes included.
The new approach implemented here handles the various cases correctly, and ensures that we do not remove a trailing slash if one was provided.