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

Spurious initial slash in empty relative path when calling .path() accessor #82

Closed
wants to merge 2 commits into from

Conversation

djcsdy
Copy link
Contributor

@djcsdy djcsdy commented Apr 23, 2013

URI("/a/b/c/").
    relativeTo("/a/b/c/").
    toString() === "";
// Good!

URI("/a/b/c/").
    relativeTo("/a/b/c/").
    path() === "/";
// Bad!

URI("").toString() === ""; // Good!

URI("").path() === "/"; // Bad!
  • 8d0c31c adds test cases demonstrating this problem.
  • 8049654 fixes the problem.

Please see the commit logs for rationale behind the changes made.

…y uri.path() and uri.pathname().

Clearly the relative path from"/a/b/c/" to "/a/b/c/" is the empty relative path "", and not "/".

Equivalently, URI("").path() should return the empty string "", but in fact returns "/".

The empty relative path is serialized correctly by .toString(), but incorrectly by .path() and .pathname().
If the URI includes an authority component, then the path is always absolute. Otherwise, the path may be relative and we should not coerce an empty relative path to the empty absolute path.

We check for the presence of a hostname as a proxy for checking for an authority component, because in our implementation the presence of a hostname determines the presence of an authority component.

There was previously a check to see if the URI was a URN, but that is now redundant since a URN cannot include an authority component.
@rodneyrehm
Copy link
Member

What's the status on this? are there still things missing? I've started merging the open stuff into master

@djcsdy
Copy link
Contributor Author

djcsdy commented Aug 3, 2013

I believe this is safe to merge.

@rodneyrehm
Copy link
Member

I've merged this into master - it will be included in the next release. thank you for your help!

@rodneyrehm rodneyrehm closed this Aug 3, 2013
@djcsdy djcsdy deleted the spurious-initial-slash branch August 3, 2013 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants