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

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

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 02612e36f3506afb694658cd4f7fd1ca14b4d656..f095ac7cf8534247479c240a6e1dc5475b7b00ad 100644 (file)
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -415,14 +415,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
@@ -435,6 +440,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
@@ -472,11 +479,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 e13825082aa9d2aae52eb51c0403e76376dc75a2..966a256b51a0e10ea7155c8653bfb42669004fa4 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -1415,26 +1415,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 2dd0d9e000657cb4f678a25aa726477b6f31853e..360ea14b1cf43538a71dc09b89b1bddf723baca9 100644 (file)
--- a/test/functional/timelog_controller_test.rb
+++ b/test/functional/timelog_controller_test.rb
@@ -173,6 +173,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 072c05102d456b57ff38316ce9ac1000de796453..65c373874b60443b0aa67116211f314b97f26c97 100644 (file)
--- a/test/helpers/application_helper_test.rb
+++ b/test/helpers/application_helper_test.rb
@@ -1862,11 +1862,6 @@ class ApplicationHelperTest < Redmine::HelperTest
     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'