search

jsmitty12 / phpWhois

phpWhois general repository
GNU General Public License v2.0
56 stars 20 forks source link

Add support for .app domains #27

Closed fideloper closed 6 years ago

fideloper commented 6 years ago

Using generic_parser_b to add support for Google's .app domains.

fideloper commented 6 years ago

I'm not sure how this testsuite works - is there an entry to testsuite.txt I should add for a .app domain?

jsmitty12 commented 6 years ago

Thanks for your PR!

I'm not sure how this testsuite works - is there an entry to testsuite.txt I should add for a .app domain?

The trick here will be finding legitimate .app domains that are likely to exist forever. Several domains were added to the test suite at one point and they no longer exist now, which causes some of the tests to fail (well, at least they're useless). I've starting using google.tld as a test domain assuming that Google will be around forever and that they'll register a domain on (nearly) every tld. (Ideally I would like to have two domains to test for each tld, but I've had trouble finding domains that I'm confident will still exist in 5-10 years.)

testsuite.txt is part of the original test suite (which I'm slowly trying to eliminate) which operates on live domains, and I don't mind if that gets updated. You need to add a new entry to the test.txt file in the Other section... something like this:

za.net      sex.za.net
app     google.app

The new pattern I'm taking can be seen by looking in the /tests directory. You can use any of the existing handler tests as a pattern. Don't forget to make a fixture by capturing the whois information to a text file in the tests/fixtures directory so the tests will work without having to make a live query.

fideloper commented 6 years ago

Thanks, I'll check this out soon!

fideloper commented 6 years ago

I searched for .app domains that google has reserved, and they use https://get.app/, so that looks like a good candidate to test with. (And google.app does have a WHOIS entry so that should work).

fideloper commented 6 years ago

Tests added within /tests dir 🤟🏻

jsmitty12 commented 6 years ago

I searched for .app domains that google has reserved, and they use https://get.app/, so that looks like a good candidate to test with. (And google.app does have a WHOIS entry so that should work).

I agree that get.app is a good candidate--its data looks a different from google.app, so let's do both!

I also noticed that most of the data is returned as "REDACTED FOR PRIVACY", which makes testing a little less definite if we start checking specific values. How are you searching for .app domains? Do you think one might exist that isn't mostly "REDACTED FOR PRIVACY"?

fideloper commented 6 years ago

So for some of this feedback, your suggestions (while I'm happy to do them!) are inconsistent with the rest of the code.

Let me know if you want to apply those changes anyway, I'm more than happy to!

jsmitty12 commented 6 years ago

The tests look great--thank you for this addition! I'll accept the PR once you've finished the code style updates.

fideloper commented 6 years ago

Think I got everything updated there - thanks for your time and feedback!