From e547d13a51168f553540e824f0a5e478fc0d496d Mon Sep 17 00:00:00 2001
From: Bob Aman <bob@sporkmonger.com>
Date: Wed, 8 Oct 2008 12:17:32 -0400
Subject: [PATCH] A sane exception is now raised for invalid Location header values.

---
 activeresource/lib/active_resource/base.rb       |    5 ++++-
 activeresource/lib/active_resource/connection.rb |    3 +++
 activeresource/test/base_test.rb                 |   14 +++++++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index bb28480..672fd1f 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -991,6 +991,9 @@ module ActiveResource
       def create
         returning connection.post(collection_path, encode, self.class.headers) do |response|
           self.id = id_from_response(response)
+          unless self.id
+            raise InvalidResponseError, "No id was present in response."
+          end
           load_attributes_from_response(response)
         end
       end
@@ -1003,7 +1006,7 @@ module ActiveResource
 
       # Takes a response from a typical create post and pulls the ID out
       def id_from_response(response)
-        response['Location'][/\/([^\/]*?)(\.\w+)?$/, 1]
+        (response['Location'] || "")[/\/([^\/]*?)(\.\w+)?$/, 1]
       end
 
       def element_path(options = nil)
diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb
index 273fee3..faa4d5a 100644
--- a/activeresource/lib/active_resource/connection.rb
+++ b/activeresource/lib/active_resource/connection.rb
@@ -59,6 +59,9 @@ module ActiveResource
     end
   end
 
+  # Response could not be processed by ActiveResource
+  class InvalidResponseError < ClientError; end # :nodoc:
+
   # Class to handle connections to remote web services.
   # This class is used by ActiveResource::Base to interface with REST
   # services.
diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb
index bb08098..3e75529 100644
--- a/activeresource/test/base_test.rb
+++ b/activeresource/test/base_test.rb
@@ -627,6 +627,12 @@ class BaseTest < Test::Unit::TestCase
     assert_equal '1', p.__send__(:id_from_response, resp)
   end
 
+  def test_id_from_response_without_location
+    p = Person.new
+    resp = {}
+    assert_equal nil, p.__send__(:id_from_response, resp)
+  end
+
   def test_create_with_custom_prefix
     matzs_house = StreetAddress.new(:person_id => 1)
     matzs_house.save
@@ -670,7 +676,6 @@ class BaseTest < Test::Unit::TestCase
     assert_equal person, person.reload
   end
 
-
   def test_create
     rick = Person.create(:name => 'Rick')
     assert rick.valid?
@@ -687,6 +692,13 @@ class BaseTest < Test::Unit::TestCase
     assert_raises(ActiveResource::ResourceConflict) { Person.create(:name => 'Rick') }
   end
 
+  def test_create_without_location
+    ActiveResource::HttpMock.respond_to do |mock|
+      mock.post   "/people.xml", {}, nil, 201
+    end
+    assert_raises(ActiveResource::InvalidResponseError) { Person.create(:name => 'Rick') }
+  end
+
   def test_clone
    matz = Person.find(1)
    matz_c = matz.clone
-- 
1.5.4.5