Support whois.smallregistry.net #144

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@mat813
Contributor
mat813 commented Jan 20, 2012

No description provided.

@weppos
Owner
weppos commented Jan 20, 2012

@mat813, I'm speechless. This patch is excellent, almost perfect!
I have a few minor feedback, I'm going to comment the pull request diff.

@weppos weppos commented on the diff Jan 20, 2012
lib/whois/definitions/tlds.rb
@@ -125,6 +125,10 @@
Whois::Server.define :tld, ".fk", nil, { :adapter => Whois::Server::Adapters::None }
Whois::Server.define :tld, ".fm", nil, { :web => "http://www.dot.fm/whois.html", :adapter => Whois::Server::Adapters::Web }
Whois::Server.define :tld, ".fo", "whois.ripe.net"
+# keep before .fr
@weppos
weppos Jan 20, 2012 Owner

Please avoid Ruby syntax here. Just keep a flatten list.

Whois::Server.define :tld, ". aeroport.fr", "whois.smallregistry.net"
Whois::Server.define :tld, ". avocat.fr", "whois.smallregistry.net"
...

It's a little bit more verbose, but I'm planning to convert this file into a JSON definition file and I'd like to keep it flatten. Also, this file was originally generated by a rake task (that is actually still in place).

@mat813
mat813 Jan 20, 2012 Contributor

Ok, done :-)

@weppos weppos commented on an outdated diff Jan 20, 2012
...ois/record/parser/scanners/whois.smallregistry.net.rb
+ end
+ end
+
+ %w(created expired updated).each do |date|
+ tokenizer :"scan_date_#{date}" do
+ if @input.match?(/^#{date}:\s+"(.*)"\n/) && @input.scan(/^#{date}:\s+"(.*)"\n/)
+ @ast["field:#{date}"] = DateTime.parse(@input[1].strip)
+ end
+ end
+ end
+
+ tokenizer :scan_name_servers do
+ if @input.match?(/^name_servers: $\n/) && @input.scan(/^name_servers: \n/)
+ @ast["nameservers"] = []
+ while line = @input.scan(/^- (\S+)(?: - (.*?)(?: - (.*?))?)?\n/)
+ @ast["nameservers"] << Record::Nameserver.new(@input[1], @input[2], @input[3])
@weppos
weppos Jan 20, 2012 Owner

Scanners should never use complex object. In this case, it's better to return a Hash and initialize the objects directly in the parser using the Hash as input.

# In the scanner
@ast["nameservers"] << { :name => @input[1], :ipv4 => @input[2], :ipv6 => @input[3] }

# In the parser
Array.wrap(node("nameservers")).each { |hash| Record::Nameserver.new(hash) }

The same applies to :scan_registrar.

This is a convention I'm trying to follow because I'd like to move scanners from a Ruby-based implementation to something more efficient such as Racc or Treetop.

@mat813
Contributor
mat813 commented Jan 20, 2012

Also, as a side note, I'm quite certain I covered all the possibilities the whois can output, as I also write that whois server :-)

@weppos weppos commented on an outdated diff Jan 20, 2012
...es/whois.smallregistry.net/status_registered.expected
+ should: %s == true
+
+#created_on
+ should: %s be_a(DateTime)
+ should: %s == DateTime.parse("2011-01-13T15:45:18+01:00")
+
+#expires_on
+ should: %s be_a(DateTime)
+ should: %s == DateTime.parse("2013-01-13T15:45:18+01:00")
+
+#updated_on
+ should: %s be_a(DateTime)
+ should: %s == DateTime.parse("2012-01-13T16:00:09+01:00")
+
+#registrar
+ should: %s be_a(Whois::Record::Parser::WhoisSmallregistryNet::Registrar)
@weppos
weppos Jan 20, 2012 Owner

The registrar Property should always be a Whois::Record::Registrar instance.

I know the current Whois::Record::Registrar has a limited number of properties, I will probably extend them in the future to map phone, address and more details.

@weppos
Owner
weppos commented Jan 20, 2012

Also, as a side note, I'm quite certain I covered all the possibilities the whois can output, as I also write that whois server :-)

LOL, amazing! I have been thinking about providing an open source Whois server/emitter for a long time, but this is more an idea rather than a real project for now.

About the scanner, I noticed you used a YAML output (this is something I have been thinking for a while about the Whois server). Wouldn't be easier to replace the current Scanner implementation with a YAML.load call?

@mat813
Contributor
mat813 commented Jan 20, 2012

Also, as a side note, I'm quite certain I covered all the possibilities
the whois can output, as I also write that whois server :-)

LOL, amazing! I have been thinking about providing an open source Whois
server/emitter for a long time, but this is more an idea rather than a
real project for now.

Well, a whois server is quite trivial, mine is in ruby, and I'm massively
using the to_yaml of Domain, Contact and Registrar objects to delegate the
pain of doing a nice thing.

About the scanner, I noticed you used a YAML output (this is something I
have been thinking for a while about the Whois server). Wouldn't be
easier to replace the current Scanner implementation with a YAML.load
call?

That may have been a bit faster, yes, but there are no complex objects,
just strings and arrays, so, I went with the StringScanner.

Mathieu Arnold

@mat813 mat813 Cleanup the scanner.
* The scanner should not use complex types, so, return hashes.
* The registrar should be a Whois::Record::Registrar, so, remove my home
  made one.
c51c019
@mat813
Contributor
mat813 commented Jan 20, 2012

Ok, I think I got it all, anything else ? :-)

@weppos
Owner
weppos commented Jan 20, 2012

I don't think so. I just need to download the patch and run it locally, may be I'm missing something I haven't caught on Github.

I'll probably get back to you and merge the patch tonight or tomorrow.

Thanks for your great work!

PS. I'm very curious to know if you are using the Ruby Whois library at your company and if you have any feedback about it. Feedback are always welcome!

@mat813
Contributor
mat813 commented Jan 20, 2012

Hum, I'm using the Ruby Whois library as small generic whois gateway in our rails powered intranet, beats doing something aweful like that :

<pre><%= `whois #{domain}` %></pre>

:-)

@weppos weppos closed this in e3f8aee Jan 30, 2012
@weppos
Owner
weppos commented Jan 30, 2012

I merged the branch, thank you.

I'm sorry, it took a little bit longer than promised. I had to make some changes to the branch (there were some coding conventions I missed when reviewing the branch).

I also decided to add a YAML-based parser. It drastically reduced the amount of custom tokenizers I had to maintain.

@mat813
Contributor
mat813 commented Jan 30, 2012

Hum, you should have told me I would have changed the code :-)

@weppos
Owner
weppos commented Jan 30, 2012

You're right, but I discovered them while I was merging the code and it was faster for me to fix them immediately. I committed each change separately so that it's easier to review and comment the changes.

@weppos weppos was assigned Feb 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment