Merged r19486 to 19488 to 4.0-stable (#32850).

git-svn-id: http://svn.redmine.org/redmine/branches/4.0-stable@19648 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 94f4359481b630648aa7130fcff3edd2786fdddc..bcad388a8864dc764b7a56bb6c718201a21c6c79 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -397,14 +397,19 @@ class ApplicationController < ActionController::Base
     url = params[:back_url]
     if url.nil? && referer = request.env['HTTP_REFERER']
       url = CGI.unescape(referer.to_s)
+      # URLs that contains the utf8=[checkmark] parameter added by Rails are
+      # parsed as invalid by URI.parse so the redirect to the back URL would
+      # not be accepted (ApplicationController#validate_back_url would return
+      # false)
+      url.gsub!(/(\?|&)utf8=\u2713&?/, '\1')
     end
     url
   end
+  helper_method :back_url
 
   def redirect_back_or_default(default, options={})
-    back_url = params[:back_url].to_s
-    if back_url.present? && valid_url = validate_back_url(back_url)
-      redirect_to(valid_url)
+    if back_url = validate_back_url(params[:back_url].to_s)
+      redirect_to(back_url)
       return
     elsif options[:referer]
       redirect_to_referer_or default
@@ -417,6 +422,8 @@ class ApplicationController < ActionController::Base
   # Returns a validated URL string if back_url is a valid url for redirection,
   # otherwise false
   def validate_back_url(back_url)
+    return false if back_url.blank?
+
     if CGI.unescape(back_url).include?('..')
       return false
     end
@@ -454,11 +461,13 @@ class ApplicationController < ActionController::Base
     return path
   end
   private :validate_back_url
+  helper_method :validate_back_url
 
   def valid_back_url?(back_url)
     !!validate_back_url(back_url)
   end
   private :valid_back_url?
+  helper_method :valid_back_url?
 
   # Redirects to the request referer if present, redirects to args or call block otherwise.
   def redirect_to_referer_or(*args, &block)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 542c2e800ffb2220a36144293469d25ecefc5256..fa2709377ca6b3e20bec6494fbe971e8ae414c2a 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -1292,26 +1292,13 @@ module ApplicationHelper
     arg.to_json.to_s.gsub('/', '\/').html_safe
   end
 
-  def back_url
-    url = params[:back_url]
-    if url.nil? && referer = request.env['HTTP_REFERER']
-      url = CGI.unescape(referer.to_s)
-      # URLs that contains the utf8=[checkmark] parameter added by Rails are
-      # parsed as invalid by URI.parse so the redirect to the back URL would
-      # not be accepted (ApplicationController#validate_back_url would return
-      # false)
-      url.gsub!(/(\?|&)utf8=\u2713&?/, '\1')
-    end
-    url
-  end
-
   def back_url_hidden_field_tag
-    url = back_url
+    url = validate_back_url(back_url)
     hidden_field_tag('back_url', url, :id => nil) unless url.blank?
   end
 
   def cancel_button_tag(fallback_url)
-    url = back_url.blank? ? fallback_url : back_url
+    url = validate_back_url(back_url) || fallback_url
     link_to l(:button_cancel), url
   end
 

diff --git a/test/functional/application_controller_test.rb b/test/functional/application_controller_test.rb
new file mode 100644 (file)
index 0000000..abaa4a2
--- /dev/null
+++ b/test/functional/application_controller_test.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+# Redmine - project management software
+# Copyright (C) 2006-2020  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require File.expand_path('../../test_helper', __FILE__)
+
+class ApplicationControllerTest < Redmine::ControllerTest
+  def test_back_url_should_remove_utf8_checkmark_from_referer
+    @request.set_header 'HTTP_REFERER', "/path?utf8=\u2713&foo=bar"
+    assert_equal "/path?foo=bar", @controller.back_url
+  end
+end

diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb
index a75c8b24676a9f5af5d0336813060647c426efb7..11d697137557ab9dea2599a4d1488ba6f96d594f 100644 (file)
--- a/test/functional/timelog_controller_test.rb
+++ b/test/functional/timelog_controller_test.rb
@@ -138,6 +138,19 @@ class TimelogControllerTest < Redmine::ControllerTest
     assert_select 'select[name=?]', 'time_entry[project_id]'
   end
 
+  def test_get_edit_should_validate_back_url
+    @request.session[:user_id] = 2
+
+    get :edit, :params => {:id => 2, :project_id => nil, :back_url => '/valid'}
+    assert_response :success
+    assert_select 'a[href=?]', '/valid', {:text => 'Cancel'}
+
+    get :edit, :params => {:id => 2, :project_id => nil, :back_url => 'invalid'}
+    assert_response :success
+    assert_select 'a[href=?]', 'invalid', {:text => 'Cancel', :count => 0}
+    assert_select 'a[href=?]', '/projects/ecookbook/time_entries', {:text => 'Cancel'}
+  end
+
   def test_post_create
     @request.session[:user_id] = 3
     assert_difference 'TimeEntry.count' do

diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb
index 70f77ec4277a62e977068ee367530e43b58348de..0cfafd2218f6579b747f6b31678d6aed23768e29 100644 (file)
--- a/test/helpers/application_helper_test.rb
+++ b/test/helpers/application_helper_test.rb
@@ -1790,11 +1790,6 @@ RAW
     assert !result.html_safe?
   end
 
-  def test_back_url_should_remove_utf8_checkmark_from_referer
-    stubs(:request).returns(stub(:env => {'HTTP_REFERER' => "/path?utf8=\u2713&foo=bar"}))
-    assert_equal "/path?foo=bar", back_url
-  end
-
   def test_hours_formatting
     set_language_if_valid 'en'