diff --git a/CHANGELOG.md b/CHANGELOG.md index 41658f5d..26b548c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ calling `super` * Serialize the `last_request_at` entry as an Integer * Ensure registration controller block yields happen on failure in addition to success (by @dpehrson) + * Storing location only for valid url (by @parallel588) ### 3.2.4 diff --git a/lib/devise/controllers/store_location.rb b/lib/devise/controllers/store_location.rb index b249037a..b9102ec1 100644 --- a/lib/devise/controllers/store_location.rb +++ b/lib/devise/controllers/store_location.rb @@ -33,14 +33,19 @@ module Devise # def store_location_for(resource_or_scope, location) session_key = stored_location_key_for(resource_or_scope) - if location - uri = URI.parse(location) + if (uri = parse_uri(location)) session[session_key] = [uri.path.sub(/\A\/+/, '/'), uri.query].compact.join('?') end end private + def parse_uri(location) + location && URI.parse(location) + rescue URI::InvalidURIError + nil + end + def stored_location_key_for(resource_or_scope) scope = Devise::Mapping.find_scope!(resource_or_scope) "#{scope}_return_to" diff --git a/test/controllers/helpers_test.rb b/test/controllers/helpers_test.rb index 4a7b6484..7ef4265e 100644 --- a/test/controllers/helpers_test.rb +++ b/test/controllers/helpers_test.rb @@ -193,6 +193,12 @@ class ControllerAuthenticatableTest < ActionController::TestCase assert_equal "/foo.bar", @controller.stored_location_for(:user) end + test 'store bad location for stores a location to redirect back to' do + assert_nil @controller.stored_location_for(:user) + @controller.store_location_for(:user, "/foo.bar\">Carry") + assert_nil @controller.stored_location_for(:user) + end + test 'store location for accepts a resource as argument' do @controller.store_location_for(User.new, "/foo.bar") assert_equal "/foo.bar", @controller.stored_location_for(User.new)