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

Modifying URI should update all _parts? #58

Closed
Jellyfrog opened this issue Jan 5, 2013 · 4 comments
Closed

Modifying URI should update all _parts? #58

Jellyfrog opened this issue Jan 5, 2013 · 4 comments

Comments

@Jellyfrog
Copy link

While validating and fixing user input (which is far from valid URIs always) I've come to this problem

Consider the following:

var uri = new URI('ExAmPle.Org:80');
if(!uri.protocol()) {
  uri.protocol('http');
}

It will indeed add the protocol http to both the _string and in _parts
BUT it will not rebuild the rest of the _parts
So a call to normalize() will fail since _parts.hostname === null

_parts will look like this after uri.protocol('http'):

duplicateQueryParameters: false
fragment: null
hostname: null
password: null
path: "ExAmPle.Org:80"
port: null
protocol: "http"
query: null
urn: null
username: null

When it should look like this:

duplicateQueryParameters: false
fragment: null
hostname: "ExAmPle.Org"
password: null
path: "/"
port: "80"
protocol: "http"
query: null
urn: null
username: null

It might not be a great idea (performance-wise) to re-parse the URI every time it is modified
so other ideas are welcome.

...Or another solution to this.

@rodneyrehm
Copy link
Member

What you want to achieve is converting something that is parsed as a relative URI (because protocol is missing) to an absolute URI. The way you described is not the way to go about this, as modifying the "atomic component" protocol would also alter other components (hostname, port, path, …).

I would prefer something along the lines of

URI.absoluteUri = function(uri) {
  if (!uri.match(/^[a-z][a-z0-9-+-]*:\/\//i)) {
    uri = "http://" + uri;
  }

  return URI(uri);
};

URI.absoluteUri('ExAmPle.Org:80').toString();

I'm not sure if this needs to be a core function, though. Are you performing any other operations to sanitize user input?

@Jellyfrog
Copy link
Author

Maybe a rebuild() function, but at the same time this feels a bit weird too.
Users might get confused since _string looks OK (after protocol('http')) but _parts still look wrong,
This maybe should be stated in the docs?

I was thinking that I could use URI.js to sanitize/validate
What I want do is something like the following:

var uri = new URI('ExAmPle.Org:80/a/b/c.htm?a=b#foo');
if(uri.protocol() != "http" && uri.protocol() != "https") {
  uri.protocol('http');
}

uri.fragment('');
uri.normalize();

if(!uri.protocol() || !uri.hostname() || !uri.path()) {
  //Url could not be fixed / validated
  return false;
}

@rodneyrehm
Copy link
Member

While http:///some-path doesn't make sense, file:///some-path does. "foo.com" may be a directory, not necessarily a domain. So there can't be any protocol() magic. That means we can't plug this into any of the existing functions.

You can do a manual re-parse any time though:

var uri = new URI('ExAmPle.Org:80/a/b/c.htm?a=b#foo');
if (!uri.protocol()) {
  uri.protocol('http');
}

if (!uri.protocol().match(/^https?$/i)) {
  return false;
}

uri.fragment('');
// force a re-parse
uri.href(uri.href());
uri.normalize();

if(!uri.protocol() || !uri.hostname() || !uri.path()) {
  //Url could not be fixed / validated
  return false;
}

@Jellyfrog
Copy link
Author

👍
I went with a implementation like you suggested, must have missed href() yesterday when I looked in the docs.
Maybe there should be a part in the docs about "invalid URIs"?
Since (like I said in the first post) its a bit confusing that the _string is OK but _parts is wrong

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

No branches or pull requests

2 participants