From 882d87327b41577ec4e0aa10435237cb691ffc39 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 5 Apr 2020 08:04:27 +0000 Subject: [PATCH] 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 --- app/controllers/application_controller.rb | 15 ++++++++--- app/helpers/application_helper.rb | 17 ++---------- .../functional/application_controller_test.rb | 27 +++++++++++++++++++ test/functional/timelog_controller_test.rb | 13 +++++++++ test/helpers/application_helper_test.rb | 5 ---- 5 files changed, 54 insertions(+), 23 deletions(-) create mode 100644 test/functional/application_controller_test.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 02612e36f..f095ac7cf 100644 --- 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 e13825082..966a256b5 100644 --- 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 index 000000000..abaa4a23e --- /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 2dd0d9e00..360ea14b1 100644 --- 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 072c05102..65c373874 100644 --- 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' -- 2.39.5