diff --git a/app/models/repository/subversion.rb b/app/models/repository/subversion.rb index 030bdb46f..324e7da8d 100644 --- a/app/models/repository/subversion.rb +++ b/app/models/repository/subversion.rb @@ -21,7 +21,7 @@ require 'redmine/scm/adapters/subversion_adapter' class Repository::Subversion < Repository validates_presence_of :url - validates_format_of :url, :with => %r{\A(http|https|svn(\+[^\s:\/\\]+)?|file):\/\/.+}i + validates_format_of :url, :with => %r{\A(http|https|svn(\+[^\s:\/\\]+)?|file):\/\/.+\z}i def self.scm_adapter_class Redmine::Scm::Adapters::SubversionAdapter diff --git a/test/functional/repositories_controller_test.rb b/test/functional/repositories_controller_test.rb index c7d36cda6..93f4e0144 100644 --- a/test/functional/repositories_controller_test.rb +++ b/test/functional/repositories_controller_test.rb @@ -120,6 +120,31 @@ class RepositoriesControllerTest < Redmine::RepositoryControllerTest end end + def test_create_should_reject_subversion_url_with_newline_injection + @request.session[:user_id] = 1 + [ + "file:///test\nfoo", + "svn+ssh://example.com/repo\r\nbar" + ].each do |injected_url| + assert_no_difference 'Repository.count', "expected #{injected_url.inspect} to be rejected" do + post( + :create, + :params => { + :project_id => 'subproject1', + :repository_scm => 'Subversion', + :repository => { + :url => injected_url, + :is_default => '1', + :identifier => '' + } + } + ) + end + assert_response :success + assert_select_error /URL is invalid/ + end + end + def test_edit @request.session[:user_id] = 1 get(:edit, :params => {:id => 11}) diff --git a/test/unit/repository_subversion_test.rb b/test/unit/repository_subversion_test.rb index dfdf520e7..3d47007ae 100644 --- a/test/unit/repository_subversion_test.rb +++ b/test/unit/repository_subversion_test.rb @@ -35,14 +35,19 @@ class RepositorySubversionTest < ActiveSupport::TestCase def test_invalid_url set_language_if_valid 'en' - ['invalid', 'http://', 'svn://', 'svn+ssh://', 'file://'].each do |url| + invalid_urls = [ + 'invalid', 'http://', 'svn://', 'svn+ssh://', 'file://', + "http://valid\nfoo", + "svn://valid\r\nbar" + ] + invalid_urls.each do |url| repo = Repository::Subversion.new( :project => @project, :identifier => 'test', :url => url ) - assert !repo.save + assert !repo.save, "expected #{url.inspect} to be rejected" assert_equal ["is invalid"], repo.errors[:url] end end