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

Handle scheme relative URIs #19

Closed

Conversation

byroot
Copy link

@byroot 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

@rodneyrehm
Copy link
Member

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
Copy link
Author

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

@rodneyrehm
Copy link
Member

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
Copy link
Author

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

@rodneyrehm
Copy link
Member

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
Copy link
Author

byroot commented Mar 18, 2012

Great ! Thanks !

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

… and it's released

@rodneyrehm rodneyrehm closed this Mar 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants