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

getRegisterable() doesn't always return the registerable domain only #1

Closed
artyuum opened this issue Sep 18, 2020 · 9 comments
Closed
Assignees

Comments

@artyuum
Copy link

artyuum commented Sep 18, 2020

Steps to reproduce

$domain = new Domain('http://facebook.com');
$domain2 = new Domain('http://www.facebook.com');

$domain->getRegisterable()); // 'http://facebook.com'
$domain2->getRegisterable()); // 'facebook.com'
@eldadfux
Copy link
Member

@artyuum please not the library constructor is expecting a domain or hostname as the first argument and not a URL.

@eldadfux
Copy link
Member

You can try and use it this way:

        $domain = new Domain(parse_url('https://www.facebook.com', PHP_URL_HOST));
       
        $this->assertEquals('www.facebook.com', $domain->get());
        $this->assertEquals('com', $domain->getTLD());
        $this->assertEquals('com', $domain->getSuffix());
        $this->assertEquals('facebook.com', $domain->getRegisterable());
        $this->assertEquals('facebook', $domain->getName());
        $this->assertEquals('www', $domain->getSub());
        $this->assertEquals(true, $domain->isKnown());
        $this->assertEquals(true, $domain->isICANN());
        $this->assertEquals(false, $domain->isPrivate());
        $this->assertEquals(false, $domain->isTest());
        
        $domain = new Domain(parse_url('https://facebook.com', PHP_URL_HOST));
       
        $this->assertEquals('facebook.com', $domain->get());
        $this->assertEquals('com', $domain->getTLD());
        $this->assertEquals('com', $domain->getSuffix());
        $this->assertEquals('facebook.com', $domain->getRegisterable());
        $this->assertEquals('facebook', $domain->getName());
        $this->assertEquals('', $domain->getSub());
        $this->assertEquals(true, $domain->isKnown());
        $this->assertEquals(true, $domain->isICANN());
        $this->assertEquals(false, $domain->isPrivate());
        $this->assertEquals(false, $domain->isTest());

@artyuum
Copy link
Author

artyuum commented Sep 18, 2020

Yup, that's the workaround I found in order to strip the "http(s)://" part. The problem is that no error/exception were returned. Why not use parse_url() internally? Or maybe this is out of the scope of this library?

@eldadfux
Copy link
Member

I think detecting if the value is a URL, hostname, or domain might be out of scope, and add unwanted complexity.

I can add a small validation for strings that starts with https:// or http:// and throw an exception because this might be a common mistake.

@artyuum
Copy link
Author

artyuum commented Sep 18, 2020

I can add a small validation for strings that starts with https:// or http:// and throw an exception because this might be a common mistake.

Agreed.

@eldadfux
Copy link
Member

@artyuum, I just pushed a fix, and released a new tag (0.2.2).

image

@artyuum
Copy link
Author

artyuum commented Sep 18, 2020

Thank you.

@artyuum artyuum closed this as completed Sep 18, 2020
@kwadx
Copy link

kwadx commented Feb 1, 2022

Hi,

I have had the case of domains starting with "http" (exemple : httpmydomain.com) so the check would fail even if the domain is perfectly valid.

I guess you should check at least for "http:" and "https:"

What do you think ?

@Meldiron
Copy link
Contributor

Meldiron commented Jun 9, 2022

This has been fixed and released 🥳 Dont hesitate to reopen issue if you still encounter same problem after upgrading.

@Meldiron Meldiron closed this as completed Jun 9, 2022
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

4 participants