Detect infinite redirects and raise a Webrat::InfiniteRedirectError (Daniel Lucraft)

History.txt

@@ -1,3 +1,9 @@
+== Git
+
+* Minor enhancements
+
+  * Detect infinite redirects and raise a Webrat::InfiniteRedirectError (Daniel Lucraft)
+  
 == 0.4.1 / 2009-01-31
 
 * Minor enhancements

lib/webrat/core/configuration.rb

@@ -51,6 +51,10 @@ module Webrat
     # Set the key that Selenium uses to determine the browser running. Default *firefox
     attr_accessor :selenium_browser_key
 
+    # How many redirects to the same URL should be halted as an infinite redirect
+    # loop? Defaults to 10
+    attr_accessor :infinite_redirect_limit
+    
     def initialize # :nodoc:
       self.open_error_files = true
       self.parse_with_nokogiri = !Webrat.on_java?
@@ -58,6 +62,7 @@ module Webrat
       self.application_port = 3001
       self.application_address = 'localhost'
       self.selenium_server_port = 4444
+      self.infinite_redirect_limit = 10
       self.selenium_browser_key = '*firefox'
     end
 

lib/webrat/core/session.rb

@@ -9,6 +9,9 @@ module Webrat
   class PageLoadError < WebratError
   end
 
+  class InfiniteRedirectError < WebratError
+  end
+  
   def self.session_class
     case Webrat.configuration.mode
     when :rails
@@ -112,11 +115,30 @@ For example:
       @http_method  = http_method
       @data         = data
 
-      request_page(response_location, :get, {}) if internal_redirect?
+      if internal_redirect?
+        check_for_infinite_redirects
+        request_page(response_location, :get, {})
+      end
 
       return response
     end
+    
+    def check_for_infinite_redirects
+      if current_url == response_location
+        @_identical_redirect_count ||= 0
+        @_identical_redirect_count += 1
+      end
+      
+      if infinite_redirect_limit_exceeded?
+        raise InfiniteRedirectError.new("#{Webrat.configuration.infinite_redirect_limit} redirects to the same URL (#{current_url.inspect})")
+      end
+    end
 
+    def infinite_redirect_limit_exceeded?
+       Webrat.configuration.infinite_redirect_limit &&
+       (@_identical_redirect_count || 0) > Webrat.configuration.infinite_redirect_limit
+    end
+    
     def success_code? #:nodoc:
       (200..499).include?(response_code)
     end

spec/integration/rails/app/controllers/webrat_controller.rb

@@ -17,6 +17,10 @@ class WebratController < ApplicationController
     redirect_to submit_path
   end
   
+  def infinite_redirect
+    redirect_to infinite_redirect_path
+  end
+  
   def external_redirect
     redirect_to "http://google.com"
   end

spec/integration/rails/config/routes.rb

@@ -3,6 +3,7 @@ ActionController::Routing::Routes.draw do |map|
     webrat.submit             "/submit",            :action => "submit"
     webrat.internal_redirect  "/internal_redirect", :action => "internal_redirect"
     webrat.external_redirect  "/external_redirect", :action => "external_redirect"
+    webrat.infinite_redirect  "/infinite_redirect", :action => "infinite_redirect"
     
     webrat.before_redirect_form     "/before_redirect_form",    :action => "before_redirect_form"
     webrat.redirect_to_show_params  "/redirect_to_show_params", :action => "redirect_to_show_params"

spec/integration/rails/test/integration/webrat_test.rb

@@ -67,6 +67,12 @@ class WebratTest < ActionController::IntegrationTest
     assert_have_selector "h1"
   end
   
+  test "should detect infinite redirects" do
+    assert_raises Webrat::InfiniteRedirectError do
+      visit infinite_redirect_path
+    end
+  end
+  
 #  test "should be able to assert have tag" do
 #    visit root_path
 #    assert_have_tag "h1"

spec/private/core/configuration_spec.rb

@@ -24,6 +24,11 @@ describe Webrat::Configuration do
     config = Webrat::Configuration.new
     config.should open_error_files
   end
+  
+  it "should detect infinite redirects after 10" do
+    config = Webrat::Configuration.new
+    config.infinite_redirect_limit.should == 10
+  end
 
   it "should be configurable with a block" do
     Webrat.configure do |config|

spec/private/core/session_spec.rb

@@ -113,15 +113,6 @@ describe Webrat::Session do
       lambda { webrat_session.request_page('some url', :get, {}) }.should raise_error(Webrat::PageLoadError)
     end
 
-    it "should follow internal redirects" do
-      webrat_session.should_receive(:internal_redirect?).twice.and_return(true, false)
-      webrat_session.response.should_receive(:headers).once.and_return({ "Location" => "/newurl" })
-
-      webrat_session.request_page("/oldurl", :get, {})
-
-      webrat_session.current_url.should == "/newurl"
-    end
-
     it "should now follow external redirects" do
       webrat_session.should_receive(:internal_redirect?).and_return(false)
 

spec/public/visit_spec.rb

@@ -31,15 +31,6 @@ describe "visit" do
     lambda { fill_in "foo", :with => "blah" }.should raise_error(Webrat::WebratError)
   end
 
-  it "should follow internal redirects" do
-    webrat_session.should_receive(:internal_redirect?).twice.and_return(true, false)
-    webrat_session.response.should_receive(:headers).once.and_return({ "Location" => "/newurl" })
-
-    visit("/oldurl")
-
-    current_url.should == "/newurl"
-  end
-
   it "should not follow external redirects" do
     webrat_session.should_receive(:internal_redirect?).and_return(false)