Inconsistent path relativization #103

Merged
merged 6 commits into from Aug 8, 2013

Conversation

Projects
None yet
2 participants
Contributor

djcsdy commented Aug 7, 2013

Oops, we broke something in relativeTo() :(.

Since the last release, relativeTo() behaves inconsistently for relative URIs that contain an absolute path but no scheme or authority.

This works as expected:

URI("/a/b/c").relativeTo("/a/"); // -> "b/c"

This should be equivalent, but since the last release it produces a different result:

URI("http://example.org/a/b/c").scheme("").authority("").relativeTo("/a/"); // -> "/a/b/c"

Which is curious because the two URIs to be relativized are equivalent:

URI("/a/b/c").toString(); // -> "/a/b/c"
URI("http://example.org/a/b/c").scheme("").authority("").toString(); // -> "/a/b/c"

Of course, path relativization and URI relativization are fundamentally different things. A URI that contains only a path component is already relative, so it's not entirely clear what the expected behaviour of relativeTo() should be. This is the sort of thing I was talking about when I said that the definition of relativeTo() needs to be tightened up :).

Of course, using relativeTo() to relativize an absolute path used to work, so it should continue to work.

It's probably me that broke this, sorry about that. I need this functionality for a commercial project I'm working on, so I will certainly get around to fixing it sooner or later if you don't beat me to it. Possibly this week, but I'm not sure yet :).

Contributor

djcsdy commented Aug 6, 2013

What the hell, I'm working on this now.

Here's a branch with a test case: https://github.com/djcsdy/URI.js/tree/issue-103

The problem is that URI.js can internally represent missing components as either null or "".

relativeTo() does this to check if two components are equivalent:

if (relativeParts.protocol === baseParts.protocol) { ... }
// null !== ""

Two possible solutions here:

  • Always normalize empty components to either null or "".
  • Treat null and "" as equivalent for the purposes of comparing components.

The first solution seems cleaner to me but the second is simpler to implement...

Owner

rodneyrehm commented Aug 6, 2013

I used to treat null and "" as different states (for protocol only, I think). I don't think this is necessary anymore. So I'd opt for the first approach, cleaning things up

Contributor

djcsdy commented Aug 6, 2013

I pushed a fix to https://github.com/djcsdy/URI.js/tree/issue-103. It works by normalizing empty URI components to null when the user sets them by calling the corresponding setter.

There seem to be some other cases where URI parts can be set to the empty string though, and in those cases relativeTo() will still break :(.

Owner

rodneyrehm commented Aug 6, 2013

do you have the time to investigate?

Contributor

djcsdy commented Aug 6, 2013

Yeah, working on it now :).

Contributor

djcsdy commented Aug 7, 2013

I turned this issue into a pull request, which should fix this bug.

Empty URI parts are now normalized to null in all cases that I could find, except path, which is still sometimes set to the empty string, but that doesn’t seem to cause any problems.

rodneyrehm merged commit f596766 into medialize:master Aug 8, 2013

djcsdy deleted the djcsdy:issue-103 branch Aug 8, 2013

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