Permalink
Browse files

Making check_redirects_resolve be based on headers['Location'] rather…

… than the argument that the redirect_to command receives. This way it works regardless of what you pass to redirect_to (a hash, a URL, an ActiveRecord object etc.). Cleaning up a few methods in url_checker and url_selector related to skipping URLs and now also making sure we drop the procotol/host since it may be in the URL
  • Loading branch information...
1 parent 69fd7bb commit 1b307b5b791056d09cbee8c59c8ee34823406baa Peter Marklund committed May 20, 2009
Showing with 61 additions and 19 deletions.
  1. +16 −12 lib/url_checker.rb
  2. +27 −7 lib/url_selector.rb
  3. +10 −0 test/controller_test.rb
  4. +8 −0 test/test_helper.rb
View
@@ -19,16 +19,10 @@ def check_urls_resolve
end
end
- def check_redirects_resolve
- if response.status =~ /302/
- if response.redirected_to.is_a?(Hash)
- # redirected_to={:action=>"foobar"}
- url = url_from_params(response.redirected_to)
- else
- # redirected_to="http://test.host/foobar"
- url = response.redirected_to[%r{#{Regexp.escape(request.host)}(.+)$}, 1]
- end
- check_url_resolves(url)
+ def check_redirects_resolve
+ redirect_url = response.headers['Location']
+ if response.status =~ /302/ && redirect_url.present?
+ check_url_resolves(redirect_url)
end
end
@@ -38,12 +32,16 @@ def urls_to_check
end
def check_url_resolves(url)
- return if url.blank? or skip_url?(url) or external_http?(url)
+ return if skip_url?(url, root_url) || external_http?(url, root_url)
url = strip_anchor(remove_query(make_absolute(url)))
return if public_file_exists?(url)
check_action_exists(url)
end
+ def root_url
+ request.protocol + request.host_with_port
+ end
+
def public_file_exists?(url)
public_path = File.join(rails_public_path, url)
File.exists?(public_path) || File.exists?(public_path + ".html")
@@ -53,14 +51,20 @@ def rails_public_path
File.join(RAILS_ROOT, "public")
end
- # Make relative URLs absolute, i.e. relative to the site root
+ # Make URLs absolute paths, i.e. relative to the site root
def make_absolute(url)
+ url = remove_host(url) if has_protocol?(url)
return url if url =~ %r{^/}
current_url = request.request_uri || url_from_params
current_url = File.dirname(current_url) if current_url !~ %r{/$}
url = File.join(current_url, url)
end
+ def remove_host(url)
+ url_no_host = url[%r{^[a-z]+://[^/]+(/.+)$}, 1]
+ url_no_host.blank? ? "/" : url_no_host
+ end
+
def remove_query(url)
url =~ /\?/ ? url[/^(.+?)\?/, 1] : url
end
View
@@ -1,16 +1,36 @@
module Html
module Test
module UrlSelector
- def skip_url?(url)
- return true if url.blank? or (!external_http?(url) and url =~ /:\/\//) # Unsupported protocol
- [/^javascript:/, /^mailto:/, /^\#$/].each do |pattern|
- return true if url =~ pattern
+ def skip_url?(url, root_url = nil)
+ if url.blank? || unsupported_protocol?(url) || special_url?(url)
+ true
+ else
+ false
end
- false
end
- def external_http?(url)
- url =~ /^http(?:s)?:\/\//
+ def special_url?(url)
+ [/^javascript:/, /^mailto:/, /^\#$/].any? { |pattern| url =~ pattern }
+ end
+
+ def external_http?(url, root_url = nil)
+ if root_url
+ http_protocol?(url) && !url.starts_with?(root_url)
+ else
+ http_protocol?(url)
+ end
+ end
+
+ def http_protocol?(url)
+ url =~ %r{^http(?:s)?://} ? true : false
+ end
+
+ def has_protocol?(url)
+ url =~ %r{^[a-z]+://} ? true : false
+ end
+
+ def unsupported_protocol?(url)
+ has_protocol?(url) && !http_protocol?(url)
end
def anchor_urls
View
@@ -82,6 +82,16 @@ def test_redirect_valid_action
assert_response :redirect
end
+ def test_redirect_external
+ get :redirect_external
+ assert_response :redirect
+ end
+
+ def test_redirect_valid_with_host
+ get :redirect_valid_with_host
+ assert_response :redirect
+ end
+
def test_image_file_exists
get :image_file_exists
assert_response :success
View
@@ -30,6 +30,14 @@ def redirect_valid_action
redirect_to :action => 'valid'
end
+ def redirect_external
+ redirect_to "http://google.com/foobar"
+ end
+
+ def redirect_valid_with_host
+ redirect_to "http://test.host/test/valid"
+ end
+
def image_file_exists
render :text => %Q{<img src="/image.jpg?23049829034"/>}
end

0 comments on commit 1b307b5

Please sign in to comment.