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

Projects
None yet
2 participants
@djcsdy
Contributor

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.

djcsdy added some commits Apr 23, 2013

Add tests demonstrating incorrect empty relative path serialization b…
…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().
Fix incorrect serialization of empty relative paths.
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

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Aug 3, 2013

Member

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

Member

rodneyrehm commented Aug 3, 2013

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

@djcsdy

This comment has been minimized.

Show comment
Hide comment
@djcsdy

djcsdy Aug 3, 2013

Contributor

I believe this is safe to merge.

Contributor

djcsdy commented Aug 3, 2013

I believe this is safe to merge.

@rodneyrehm

This comment has been minimized.

Show comment
Hide comment
@rodneyrehm

rodneyrehm Aug 3, 2013

Member

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

Member

rodneyrehm commented Aug 3, 2013

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 djcsdy:spurious-initial-slash branch Aug 3, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment