PublicSuffixService.valid? should return false if the domain is not d…

…efined or not allowed (closes #4, closes #5)

CHANGELOG.rdoc

@@ -6,6 +6,8 @@
 
 * FIXED: RuleList cache is not recreated when a new rule is appended to the list (#6)
 
+* FIXED: PublicSuffixService.valid? should return false if the domain is not defined or not allowed (#4, #5)
+
 
 == Release 0.6.0
 

Rakefile

@@ -121,7 +121,7 @@ begin
     desc "Publish YARD documentation to the site"
     task :publish => ["yardoc:clobber", "yardoc"] do
       ENV["username"] || raise(ArgumentError, "Missing ssh username")
-      sh "rsync -avz --delete yardoc/ #{ENV["username"]}@code:/var/www/apps/code/#{PKG_NAME}/yardoc"
+      sh "rsync -avz --delete yardoc/ #{ENV["username"]}@code:/var/www/apps/code/#{PKG_NAME}/api"
     end
 
     desc "Remove YARD products"

lib/public_suffix_service.rb

@@ -56,14 +56,20 @@ module PublicSuffixService
   #
   # @raise [PublicSuffixService::Error]
   #   If domain is not a valid domain.
+  # @raise [PublicSuffixService::DomainNotAllowed]
+  #   If a rule for +domain+ is found, but the rule
+  #   doesn't allow +domain+.
   #
   def self.parse(domain)
-    rule = RuleList.default.find(domain) || raise(DomainInvalid, "`#{domain}' is not a valid domain")
+    rule = RuleList.default.find(domain)
+    if rule.nil?
+      raise(DomainInvalid, "`#{domain}' is not a valid domain")
+    end
+    if !rule.allow?(domain)
+      raise DomainNotAllowed, "`#{domain}' is not allowed according to Registry policy"
+    end
 
     left, right = rule.decompose(domain)
-    if right.nil?
-      raise DomainNotAllowed, "Rule `#{rule.name}' doesn't allow `#{domain}'"
-    end
 
     parts = left.split(".")
     # If we have 0 parts left, there is just a tld and no domain or subdomain
@@ -76,7 +82,7 @@ def self.parse(domain)
     Domain.new(tld, sld, trd)
   end
 
-  # Checks whether +domain+ is a valid domain name,
+  # Checks whether +domain+ is assigned and allowed,
   # without actually parsing it.
   #
   # This method doesn't care whether domain is a domain or subdomain.
@@ -88,23 +94,30 @@ def self.parse(domain)
   # @return [Boolean]
   #
   # @example Check a valid domain
-  #   PublicSuffixService.valid?("google.com")
+  #   PublicSuffixService.valid?("example.com")
   #   # => true
   #
   # @example Check a valid subdomain
-  #   PublicSuffixService.valid?("www.google.com")
+  #   PublicSuffixService.valid?("www.example.com")
   #   # => true
   #
-  # @example Check an invalid domain
-  #   PublicSuffixService.valid?("x.yz")
+  # @example Check a not-assigned domain
+  #   PublicSuffixService.valid?("example.zip")
   #   # => false
   #
+  # @example Check a not-allowed domain
+  #   PublicSuffixService.valid?("example.do")
+  #   # => false
+  #   PublicSuffixService.valid?("www.example.do")
+  #   # => true
+  #
   # @example Check an URL (which is not a valid domain)
-  #   PublicSuffixService.valid?("http://www.google.com")
+  #   PublicSuffixService.valid?("http://www.example.com")
   #   # => false
   #
   def self.valid?(domain)
-    !RuleList.default.find(domain).nil?
+    rule = RuleList.default.find(domain)
+    !rule.nil? && rule.allow?(domain)
   end
 
 end

lib/public_suffix_service/domain.rb

@@ -303,17 +303,38 @@ def is_a_subdomain?
       subdomain?
     end
 
-    # Checks whether <tt>self</tt> is valid
+    # Checks whether <tt>self</tt> is assigned and allowed
     # according to default {RuleList}.
     #
     # This method triggers a new rule lookup in the default {RuleList},
     # which is a quite intensive task.
     #
     # @return [Boolean]
+    #
+    # @example Check a valid domain
+    #   Domain.new("com", "example").valid?
+    #   # => true
+    #
+    # @example Check a valid subdomain
+    #   Domain.new("com", "example", "www").valid?
+    #   # => true
+    #
+    # @example Check a not-assigned domain
+    #   Domain.new("zip", "example").valid?
+    #   # => false
+    #
+    # @example Check a not-allowed domain
+    #   Domain.new("do", "example").valid?
+    #   # => false
+    #   Domain.new("do", "example", "www").valid?
+    #   # => true
+    #
     def valid?
-      !rule.nil?
+      r = rule
+      !r.nil? && r.allow?(name)
     end
 
+
     # Checks whether <tt>self</tt> looks like a domain and validates
     # according to default {RuleList}.
     #

lib/public_suffix_service/rule.rb

@@ -167,18 +167,47 @@ def ==(other)
       alias :eql? :==
 
 
-      # Checks whether this rule matches +domain+.
+      # Checks if this rule matches +domain+.
       #
       # @param [String, #to_s] domain
       #   The domain name to check.
       #
       # @return [Boolean]
+      #
+      # @example
+      #   rule = Rule.factory("com")
+      #   # #<PublicSuffixService::Rule::Normal>
+      #   rule.match?("example.com")
+      #   # => true
+      #   rule.match?("example.net")
+      #   # => false
+      #
       def match?(domain)
         l1 = labels
         l2 = Domain.domain_to_labels(domain)
         odiff(l1, l2).empty?
       end
 
+      # Checks if this rule allows +domain+.
+      #
+      # @param [String, #to_s] domain
+      #   The domain name to check.
+      #
+      # @return [Boolean]
+      #
+      # @example
+      #   rule = Rule.factory("*.do")
+      #   # #<PublicSuffixService::Rule::Wildcard>
+      #   rule.allow?("example.do")
+      #   # => false
+      #   rule.allow?("www.example.do")
+      #   # => true
+      #
+      def allow?(domain)
+        !decompose(domain).last.nil?
+      end
+
+
       # Gets the length of this rule for comparison.
       # The length usually matches the number of rule +parts+.
       #
@@ -189,12 +218,17 @@ def length
         parts.length
       end
 
+      #
       # @raise  [NotImplementedError]
       # @abstract
       def parts
         raise NotImplementedError
       end
 
+      #
+      # @param [String, #to_s] domain
+      #   The domain name to decompose.
+      #
       # @return [Array<String, nil>]
       #
       # @raise  [NotImplementedError]

test/public_suffix_service/domain_test.rb

@@ -121,12 +121,19 @@ def test_is_a_subdomain_question
   end
 
   def test_valid_question
-    assert  @klass.new("com").valid?
-    assert  @klass.new("com", "google").valid?
-    assert  @klass.new("com", "google", "www").valid?
+    assert !@klass.new("com").valid?
+    assert  @klass.new("com", "example").valid?
+    assert  @klass.new("com", "example", "www").valid?
+
+    # not-assigned
     assert !@klass.new("zip").valid?
-    assert !@klass.new("zip", "google").valid?
-    assert !@klass.new("zip", "google", "www").valid?
+    assert !@klass.new("zip", "example").valid?
+    assert !@klass.new("zip", "example", "www").valid?
+
+    # not-allowed
+    assert !@klass.new("do").valid?
+    assert !@klass.new("do", "example").valid?
+    assert  @klass.new("do", "example", "www").valid?
   end
 
   def test_valid_domain_question

test/public_suffix_service/rule_test.rb

@@ -60,15 +60,15 @@ def test_equality_with_internals
 
 
   def test_match
-    assert  @klass.new("uk").match?("google.uk")
-    assert !@klass.new("gk").match?("google.uk")
-    assert !@klass.new("google").match?("google.uk")
-    assert  @klass.new("uk").match?("google.co.uk")
-    assert !@klass.new("gk").match?("google.co.uk")
-    assert !@klass.new("co").match?("google.co.uk")
-    assert  @klass.new("co.uk").match?("google.co.uk")
-    assert !@klass.new("uk.co").match?("google.co.uk")
-    assert !@klass.new("go.uk").match?("google.co.uk")
+    assert  @klass.new("uk").match?("example.uk")
+    assert !@klass.new("gk").match?("example.uk")
+    assert !@klass.new("example").match?("example.uk")
+    assert  @klass.new("uk").match?("example.co.uk")
+    assert !@klass.new("gk").match?("example.co.uk")
+    assert !@klass.new("co").match?("example.co.uk")
+    assert  @klass.new("co.uk").match?("example.co.uk")
+    assert !@klass.new("uk.co").match?("example.co.uk")
+    assert !@klass.new("go.uk").match?("example.co.uk")
   end
 
   def test_length
@@ -104,17 +104,24 @@ def test_initialize
 
 
   def test_match
-    assert  @klass.new("uk").match?("google.uk")
-    assert !@klass.new("gk").match?("google.uk")
-    assert !@klass.new("google").match?("google.uk")
-    assert  @klass.new("uk").match?("google.co.uk")
-    assert !@klass.new("gk").match?("google.co.uk")
-    assert !@klass.new("co").match?("google.co.uk")
-    assert  @klass.new("co.uk").match?("google.co.uk")
-    assert !@klass.new("uk.co").match?("google.co.uk")
-    assert !@klass.new("go.uk").match?("google.co.uk")
+    assert  @klass.new("uk").match?("example.uk")
+    assert !@klass.new("gk").match?("example.uk")
+    assert !@klass.new("example").match?("example.uk")
+    assert  @klass.new("uk").match?("example.co.uk")
+    assert !@klass.new("gk").match?("example.co.uk")
+    assert !@klass.new("co").match?("example.co.uk")
+    assert  @klass.new("co.uk").match?("example.co.uk")
+    assert !@klass.new("uk.co").match?("example.co.uk")
+    assert !@klass.new("go.uk").match?("example.co.uk")
   end
 
+  def test_allow
+    assert !@klass.new("com").allow?("com")
+    assert  @klass.new("com").allow?("example.com")
+    assert  @klass.new("com").allow?("www.example.com")
+  end
+
+
   def test_length
     assert_equal 1, @klass.new("com").length
     assert_equal 2, @klass.new("co.com").length
@@ -128,8 +135,8 @@ def test_parts
   end
 
   def test_decompose
-    assert_equal %w(google com), @klass.new("com").decompose("google.com")
-    assert_equal %w(foo.google com), @klass.new("com").decompose("foo.google.com")
+    assert_equal %w(example com), @klass.new("com").decompose("example.com")
+    assert_equal %w(foo.example com), @klass.new("com").decompose("foo.example.com")
   end
 
 end
@@ -153,14 +160,15 @@ def test_initialize
 
 
   def test_match
-    assert  @klass.new("!uk").match?("google.co.uk")
-    assert !@klass.new("!gk").match?("google.co.uk")
-    assert  @klass.new("!co.uk").match?("google.co.uk")
-    assert !@klass.new("!go.uk").match?("google.co.uk")
+    assert  @klass.new("!uk").match?("example.co.uk")
+    assert !@klass.new("!gk").match?("example.co.uk")
+    assert  @klass.new("!co.uk").match?("example.co.uk")
+    assert !@klass.new("!go.uk").match?("example.co.uk")
     assert  @klass.new("!british-library.uk").match?("british-library.uk")
-    assert !@klass.new("!british-library.uk").match?("google.co.uk")
+    assert !@klass.new("!british-library.uk").match?("example.co.uk")
   end
 
+
   def test_length
     assert_equal 1, @klass.new("!british-library.uk").length
     assert_equal 2, @klass.new("!foo.british-library.uk").length
@@ -197,12 +205,19 @@ def test_initialize
 
 
   def test_match
-    assert  @klass.new("*.uk").match?("google.uk")
-    assert  @klass.new("*.uk").match?("google.co.uk")
-    assert  @klass.new("*.co.uk").match?("google.co.uk")
-    assert !@klass.new("*.go.uk").match?("google.co.uk")
+    assert  @klass.new("*.uk").match?("example.uk")
+    assert  @klass.new("*.uk").match?("example.co.uk")
+    assert  @klass.new("*.co.uk").match?("example.co.uk")
+    assert !@klass.new("*.go.uk").match?("example.co.uk")
   end
 
+  def test_allow
+    assert !@klass.new("*.com").allow?("com")
+    assert !@klass.new("*.com").allow?("example.com")
+    assert  @klass.new("*.com").allow?("www.example.com")
+  end
+
+
   def test_length
     assert_equal 2, @klass.new("*.uk").length
     assert_equal 3, @klass.new("*.co.uk").length