Handle scheme relative URIs #19

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

byroot commented Mar 17, 2012

Add support for scheme relative URLs like //example.com/file.html

Also note that the URL tested here: https://github.com/medialize/URI.js/blob/gh-pages/test/urls.js#L349 is invalid and should throw an error on parsing, or at least consider the whole string as a path.

Example:

//a248.e.akamai.net/assets.github.com/images/modules/header/logov7@4x.png

Valid relative URL

://a248.e.akamai.net/assets.github.com/images/modules/header/logov7@4x.png

Invalid URL

Owner

rodneyrehm commented Mar 17, 2012

Thanks!

As I'm currently preparing the upcomming version 1.6.0, I've "manually merged" your fix locally.
1.6.0 will be released some time next week.

parsing will treat :// as // to compensate bad URI representation. Also .protocol(null) will remove the protocol, while .protocol('') will make it relative.

byroot commented Mar 17, 2012

I don"t see a case where "removing the protocol" make sense.

Because if you remove the protocol of http://example.com/foo the result will be equivalent to ./example.com/foo which is unwanted.

You should take a look at Ruby's URI class:

>> require 'uri'
>> uri = URI.parse('http://example.com/foo')
>> uri.scheme = ''
URI::InvalidComponentError: bad component(expected scheme component): 
    from /Users/byroot/.rvm/rubies/ruby-1.9.3-p0/lib/ruby/1.9.1/uri/generic.rb:329:in `check_scheme'
    from /Users/byroot/.rvm/rubies/ruby-1.9.3-p0/lib/ruby/1.9.1/uri/generic.rb:370:in `scheme='
    from (irb):3
    from /Users/byroot/.rvm/rubies/ruby-1.9.3-p0/bin/irb:16:in `<main>'
>> uri.scheme = nil
>> uri.to_s
=> "//example.com/foo"

I think both null and '' should mutate the URL as scheme-relative.

My 2 cents

Owner

rodneyrehm commented Mar 18, 2012

A protocol (scheme) without host makes sense for URLs file:///foobar and URNs mailto:mail@example.org. But a host without a protocol (scheme) will not be interpreted as such: example.org/foobar ends up being a relative path.

It is fair to assume that removing the scheme itself is never the intention. If the scheme is to be removed, the host (authority, actually) would be removed as well to have path+query+fragment as the remaining URL.

Because there are host-less protocols (file://…) we cannot infer from an empty host that the scheme must be dumped. That leaves us with the necessity to specifically remove the protocol.

Do you agree?

byroot commented Mar 18, 2012

Yeah, but I think the solution is in the buildmethod.

Currently if we have a scheme we add ://. but i think we should only add : and add // only if we have a host.
This should resolve the issue with minimum internal changes.

The algorithm will be:

URI.build = function(parts) {
    var t = '';

    if (typeof parts.protocol === "string" && parts.protocol.length) {
        t += parts.protocol + ":";
    }
    if (parts.hostname) {
        t += '//';
    }

    t += (URI.buildAuthority(parts) || '');

    if (typeof parts.path === "string") {
        if (parts.path[0] !== '/' && typeof parts.hostname === "string") {
            t += '/';
        }

        t += parts.path;
    }

    if (typeof parts.query == "string") {
        t += '?' + parts.query;
    }

    if (typeof parts.fragment === "string") {
        t += '#' + parts.fragment;
    }
    return t;
};

And it will perform like this:

URI.build({protocol: 'mailto', hostname: 'example.com', username: 'dave.null'}) // -> mailto:dave.null@example.com

URI.build({protocol: 'file', path: '/etc/hosts'}) // -> file:/etc/hosts  // not exactly file:///etc/hosts but perfectly valid and equivalent

URI.build({hostname: 'example.com', path: '/foo'}) // -> //example.com/foo
Owner

rodneyrehm commented Mar 18, 2012

I give up. null and "" will now both cause the relative-scheme. Here's the change I made:

var t = '';

if (parts.protocol) {
    t += parts.protocol + ":";
}

if (!parts.urn && (t || parts.hostname)) {
    t += '//';
}

that parts.urn thing is required to make URI.js URN compatible. But in essence, it's your approach. :)

(again, will be released some time next week)

byroot commented Mar 18, 2012

Great ! Thanks !

@rodneyrehm rodneyrehm added a commit that referenced this pull request Mar 19, 2012

@rodneyrehm rodneyrehm Fixing Issue #19 - relative scheme URLs 54a475f
Owner

rodneyrehm commented Mar 19, 2012

… and it's released

rodneyrehm closed this Mar 19, 2012

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