Treat 303 See Other as a redirect response, too

4 files changed, 29 insertions, 6 deletions

diff --git a/activeresource/CHANGELOG b/activeresource/CHANGELOG
index fe356d7691..708312e1b0 100644
--- a/activeresource/CHANGELOG
+++ b/activeresource/CHANGELOG
@@ -1,3 +1,16 @@
+*Rails 3.1.2 (unreleased)*
+
+* Redirect responses: 303 See Other and 307 Temporary Redirect now behave like
+  301 Moved Permanently and 302 Found.  GH #3302.
+
+  [Jim Herz]
+
+
+*Rails 3.1.1 (October 7, 2011)*
+
+* No changes.
+
+
 *Rails 3.1.0 (August 30, 2011)*
 
 * The default format has been changed to JSON for all requests. If you want to continue to use XML you will need to set `self.format = :xml` in the class. eg.
diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb
index 3fb8ce7790..7b8afdca3c 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -170,8 +170,8 @@ module ActiveResource
   # <tt>404</tt> is just one of the HTTP error response codes that Active Resource will handle with its own exception. The
   # following HTTP response codes will also result in these exceptions:
   #
-  # * 200..399 - Valid response, no exception (other than 301, 302 and 307)
-  # * 301, 302, 307 - ActiveResource::Redirection
+  # * 200..399 - Valid response. No exceptions, other than these redirects:
+  # * 301, 302, 303, 307 - ActiveResource::Redirection
   # * 400 - ActiveResource::BadRequest
   # * 401 - ActiveResource::UnauthorizedAccess
   # * 403 - ActiveResource::ForbiddenAccess
diff --git a/activeresource/lib/active_resource/connection.rb b/activeresource/lib/active_resource/connection.rb
index 73410c2d82..94839c8c25 100644
--- a/activeresource/lib/active_resource/connection.rb
+++ b/activeresource/lib/active_resource/connection.rb
@@ -122,7 +122,7 @@ module ActiveResource
       # Handles response and error codes from the remote service.
       def handle_response(response)
         case response.code.to_i
-          when 301,302,307
+          when 301, 302, 303, 307
             raise(Redirection.new(response))
           when 200...400
             response
diff --git a/activeresource/test/cases/connection_test.rb b/activeresource/test/cases/connection_test.rb
index 6f33023832..535107aeef 100644
--- a/activeresource/test/cases/connection_test.rb
+++ b/activeresource/test/cases/connection_test.rb
@@ -2,6 +2,7 @@ require 'abstract_unit'
 
 class ConnectionTest < Test::Unit::TestCase
   ResponseCodeStub = Struct.new(:code)
+  RedirectResponseStub = Struct.new(:code, :Location)
 
   def setup
     @conn = ActiveResource::Connection.new('http://localhost')
@@ -39,13 +40,16 @@ class ConnectionTest < Test::Unit::TestCase
     end
 
     # 301 is moved permanently (redirect)
-    assert_response_raises ActiveResource::Redirection, 301
+    assert_redirect_raises 301
 
     # 302 is found (redirect)
-    assert_response_raises ActiveResource::Redirection, 302
+    assert_redirect_raises 302
+
+    # 303 is see other (redirect)
+    assert_redirect_raises 303
 
     # 307 is temporary redirect
-    assert_response_raises ActiveResource::Redirection, 307
+    assert_redirect_raises 307
 
     # 400 is a bad request (e.g. malformed URI or missing request parameter)
     assert_response_raises ActiveResource::BadRequest, 400
@@ -256,6 +260,12 @@ class ConnectionTest < Test::Unit::TestCase
       end
     end
 
+    def assert_redirect_raises(code)
+      assert_raise(ActiveResource::Redirection, "Expected response code #{code} to raise ActiveResource::Redirection") do
+        handle_response RedirectResponseStub.new(code, 'http://example.com/')
+      end
+    end
+
     def handle_response(response)
       @conn.__send__(:handle_response, response)
     end