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 ip addresses #104

Closed
remusao opened this issue Sep 6, 2017 · 7 comments
Closed

Handle ip addresses #104

remusao opened this issue Sep 6, 2017 · 7 comments

Comments

@remusao
Copy link
Collaborator

remusao commented Sep 6, 2017

Hi,

Currently we don't detect if a url contains a hostname or an ip address. We should probably add some check so that we return a consistent result.

Current behavior:

> tldjs.parse('https://123.23.32.31')
{ valid: true,
  hostname: '123.23.32.31',
  suffix: '31',
  domain: '32.31',
  subdomain: '123.23' }

Should probably be something like:

> tldjs.parse('https://123.23.32.31')
{ valid: true,
  hostname: '123.23.32.31',
  suffix: null,
  domain: null, // Or `hostname`?
  subdomain: null }
@thom4parisot
Copy link
Owner

Oh, good thinking. I never tried with an IPv6 too but I suppose its syntax would fail on an isValid call.

@remusao
Copy link
Collaborator Author

remusao commented Sep 9, 2017

What do you think about including some dependency to do this validation? I saw two libraries looking pretty robust:

@thom4parisot
Copy link
Owner

Well, validator has no dependency, has isDataURI andisEmail (cf. #101).
But the rest is not relevant so it is less nice, bundle size wise.

PS: funnily, it has a isFQDN method 😉

@remusao
Copy link
Collaborator Author

remusao commented Sep 9, 2017

So did I understand correctly that we should use is-ip instead? Or do you think it's worth it to integrate also checks for isEmail and isDataURI?

PS: funnily, isFQDN is 10x slower than isValid :P

@thom4parisot
Copy link
Owner

Yeah let's go ahead with is-ip. We can swap it with something else if needed in the future.

PS: 😂

@remusao
Copy link
Collaborator Author

remusao commented Sep 10, 2017

@oncletom I started adding the support for ip addressed in tld.js. Will do a PR when it's ready. I might also try to switch to whatwg-url for parsing if it works out fine.

@remusao
Copy link
Collaborator Author

remusao commented Mar 16, 2018

IP addresses are now detected since #108.

@remusao remusao closed this as completed Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants