Perform acceptance check in CookieJar#add instead of origin=.

- Cookie#acceptable? is added, which is called by such methods as
  Cookie.parse and CookieJar#add.

- Cookie#origin= no longer raises ArgumentError just because it
  conflicts with the domain.

- Cookie#origin= raises ArgumentError if it is given an object that is
  not URI or string-like.

lib/http/cookie.rb

@@ -313,14 +313,9 @@ class HTTP::Cookie
             end
           }
 
-          if origin
-            begin
-              cookie.origin = origin
-            rescue => e
-              logger.warn("Invalid cookie for the origin: #{origin} (#{e})") if logger
-              next
-            end
-          end
+          cookie.origin = origin
+
+          cookie.acceptable? or next
 
           yield cookie if block_given?
 
@@ -422,10 +417,10 @@ class HTTP::Cookie
     @origin.nil? or
       raise ArgumentError, "origin cannot be changed once it is set"
     origin = URI(origin)
-    self.domain ||= origin.host
-    self.path   ||= (origin + './').path
-    acceptable_from_uri?(origin) or
-      raise ArgumentError, "unacceptable cookie sent from URI #{origin}"
+    if URI::HTTP === origin
+      self.domain ||= origin.host
+      self.path   ||= (origin + './').path
+    end
     @origin = origin
   end
 
@@ -534,6 +529,22 @@ class HTTP::Cookie
     end
   end
 
+  # Tests if it is OK to accept this cookie considering its origin.
+  # If either domain or path is missing, raises ArgumentError.  If
+  # origin is missing, returns true.
+  def acceptable?
+    case
+    when @domain.nil?
+      raise ArgumentError, "domain is missing"
+    when @path.nil?
+      raise ArgumentError, "path is missing"
+    when @origin.nil?
+      true
+    else
+      acceptable_from_uri?(@origin)
+    end
+  end
+
   # Tests if it is OK to send this cookie to a given `uri`.  A runtime
   # error is raised if the cookie's domain is unknown.
   def valid_for_uri?(uri)

lib/http/cookie_jar.rb

@@ -45,8 +45,9 @@ class HTTP::CookieJar
     @store = other.instance_eval { @store.dup }
   end
 
-  # Adds a cookie to the jar and returns self.  A given cookie must
-  # have domain and path attributes set, or ArgumentError is raised.
+  # Adds a cookie to the jar if it is acceptable, and returns self in
+  # any case.  A given cookie must have domain and path attributes
+  # set, or ArgumentError is raised.
   #
   # ### Compatibility Note for Mechanize::Cookie users
   #
@@ -63,14 +64,10 @@ class HTTP::CookieJar
   #       jar.add!(cookie)    # no acceptance check is performed
   #
   #       # HTTP::Cookie
-  #       jar.origin = origin # acceptance check is performed
-  #       jar.add(cookie)
+  #       jar.origin = origin
+  #       jar.add(cookie)     # acceptance check is performed
   def add(cookie)
-    if cookie.domain.nil? || cookie.path.nil?
-      raise ArgumentError, "a cookie with unknown domain or path cannot be added"
-    end
-
-    @store.add(cookie)
+    @store.add(cookie) if cookie.acceptable?
     self
   end
   alias << add

test/test_http_cookie.rb

@@ -450,6 +450,9 @@ class TestHTTPCookie < Test::Unit::TestCase
     assert_equal 'key', cookie.name
     assert_equal 'value', cookie.value
     assert_equal nil, cookie.expires
+    assert_raises(ArgumentError) {
+      cookie.acceptable?
+    }
 
     # Minimum unit for the expires attribute is second
     expires = Time.at((Time.now + 3600).to_i)
@@ -458,24 +461,39 @@ class TestHTTPCookie < Test::Unit::TestCase
     assert_equal 'key', cookie.name
     assert_equal 'value', cookie.value
     assert_equal expires, cookie.expires
+    assert_raises(ArgumentError) {
+      cookie.acceptable?
+    }
 
     cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup)
     assert_equal 'key', cookie.name
     assert_equal 'value', cookie.value
     assert_equal expires, cookie.expires
     assert_equal false, cookie.for_domain?
+    assert_raises(ArgumentError) {
+      # domain and path are missing
+      cookie.acceptable?
+    }
 
     cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => '.example.com')
     assert_equal 'key', cookie.name
     assert_equal 'value', cookie.value
     assert_equal expires, cookie.expires
     assert_equal true, cookie.for_domain?
+    assert_raises(ArgumentError) {
+      # path is missing
+      cookie.acceptable?
+    }
 
     cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => 'example.com', :for_domain => false)
     assert_equal 'key', cookie.name
     assert_equal 'value', cookie.value
     assert_equal expires, cookie.expires
     assert_equal false, cookie.for_domain?
+    assert_raises(ArgumentError) {
+      # path is missing
+      cookie.acceptable?
+    }
 
     cookie = HTTP::Cookie.new(:value => 'value', :name => 'key', :expires => expires.dup, :domain => 'example.org', :for_domain? => true)
     assert_equal 'key', cookie.name
@@ -483,6 +501,10 @@ class TestHTTPCookie < Test::Unit::TestCase
     assert_equal expires, cookie.expires
     assert_equal 'example.org', cookie.domain
     assert_equal true, cookie.for_domain?
+    assert_raises(ArgumentError) {
+      # path is missing
+      cookie.acceptable?
+    }
 
     assert_raises(ArgumentError) { HTTP::Cookie.new(:name => 'name') }
     assert_raises(ArgumentError) { HTTP::Cookie.new(:value => 'value') }
@@ -591,30 +613,46 @@ class TestHTTPCookie < Test::Unit::TestCase
   def test_new_rejects_cookies_that_do_not_contain_an_embedded_dot
     url = URI 'http://rubyforge.org/'
 
-    assert_raises(ArgumentError) {
-      tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url))
-    }
-    assert_raises(ArgumentError) {
-      single_dot_cookie = HTTP::Cookie.new(cookie_values(:domain => '.', :origin => url))
-    }
+    tld_cookie1 = HTTP::Cookie.new(cookie_values(:domain => 'org', :origin => url))
+    assert_equal false, tld_cookie1.for_domain?
+    assert_equal 'org', tld_cookie1.domain
+    assert_equal false, tld_cookie1.acceptable?
+
+    tld_cookie2 = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url))
+    assert_equal false, tld_cookie1.for_domain?
+    assert_equal 'org', tld_cookie2.domain
+    assert_equal false, tld_cookie2.acceptable?
+  end
+
+  def test_new_tld_domain_from_tld
+    url = URI 'http://org/'
+
+    tld_cookie1 = HTTP::Cookie.new(cookie_values(:domain => 'org', :origin => url))
+    assert_equal false, tld_cookie1.for_domain?
+    assert_equal 'org', tld_cookie1.domain
+    assert_equal true, tld_cookie1.acceptable?
+
+    tld_cookie2 = HTTP::Cookie.new(cookie_values(:domain => '.org', :origin => url))
+    assert_equal false, tld_cookie1.for_domain?
+    assert_equal 'org', tld_cookie2.domain
+    assert_equal true, tld_cookie2.acceptable?
   end
 
   def test_fall_back_rules_for_local_domains
     url = URI 'http://www.example.local'
 
-    assert_raises(ArgumentError) {
-      tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.local', :origin => url))
-    }
+    tld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.local', :origin => url))
+    assert_equal false, tld_cookie.acceptable?
 
     sld_cookie = HTTP::Cookie.new(cookie_values(:domain => '.example.local', :origin => url))
+    assert_equal true, sld_cookie.acceptable?
   end
 
   def test_new_rejects_cookies_with_ipv4_address_subdomain
     url = URI 'http://192.168.0.1/'
 
-    assert_raises(ArgumentError) {
-      cookie = HTTP::Cookie.new(cookie_values(:domain => '.0.1', :origin => url))
-    }
+    cookie = HTTP::Cookie.new(cookie_values(:domain => '.0.1', :origin => url))
+    assert_equal false, cookie.acceptable?
   end
 
   def test_path
@@ -675,11 +713,15 @@ class TestHTTPCookie < Test::Unit::TestCase
     url = URI.parse('http://example.com/path/')
 
     cookie = HTTP::Cookie.new('a', 'b')
+    assert_raises(ArgumentError) {
+      cookie.origin = 123
+    }
     cookie.origin = url
     assert_equal '/path/', cookie.path
     assert_equal 'example.com', cookie.domain
     assert_equal false, cookie.for_domain
     assert_raises(ArgumentError) {
+      # cannot change the origin once set
       cookie.origin = URI.parse('http://www.example.com/')
     }
 
@@ -689,13 +731,21 @@ class TestHTTPCookie < Test::Unit::TestCase
     assert_equal 'example.com', cookie.domain
     assert_equal true, cookie.for_domain
     assert_raises(ArgumentError) {
+      # cannot change the origin once set
       cookie.origin = URI.parse('http://www.example.com/')
     }
 
     cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com')
-    assert_raises(ArgumentError) {
-      cookie.origin = URI.parse('http://example.org/')
-    }
+    cookie.origin = URI.parse('http://example.org/')
+    assert_equal false, cookie.acceptable?
+
+    cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com')
+    cookie.origin = 'file:///tmp/test.html'
+    assert_equal nil, cookie.path
+
+    cookie = HTTP::Cookie.new('a', 'b', :domain => '.example.com', :path => '/')
+    cookie.origin = 'file:///tmp/test.html'
+    assert_equal false, cookie.acceptable?
   end
 
   def test_acceptable_from_uri?

test/test_http_cookie_jar.rb

@@ -284,9 +284,7 @@ module TestHTTPCookieJar
     def test_cookies_no_host
       url = URI 'file:///path/'
 
-      assert_raises(ArgumentError) {
-        @jar.add(HTTP::Cookie.new(cookie_values(:origin => url)))
-      }
+      @jar.add(HTTP::Cookie.new(cookie_values(:origin => url)))
 
       assert_equal(0, @jar.cookies(url).length)
     end