summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2017-03-21 17:22:27 +0000
committerDJ Mountney <david@twkie.net>2017-03-21 10:41:17 -0700
commit0f711b0818888523b400e898b19c5a2954a2613d (patch)
tree04fed02bbe7ae062495be7bf64112d2f60242340
parentc0d47ad566a40f486a82429c96be1126a6e02454 (diff)
downloadgitlab-ce-0f711b0818888523b400e898b19c5a2954a2613d.tar.gz
Merge branch '29583-routes-like-fix' into 'master'
Escape route path for LIKE queries Closes #29583 See merge request !10117
-rw-r--r--app/models/namespace.rb2
-rw-r--r--app/models/project.rb2
-rw-r--r--app/models/route.rb4
-rw-r--r--spec/models/project_spec.rb11
-rw-r--r--spec/models/route_spec.rb24
5 files changed, 36 insertions, 7 deletions
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index d350f1d6770..826ded22ae5 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -195,7 +195,7 @@ class Namespace < ActiveRecord::Base
# Scopes the model on direct and indirect children of the record
def descendants
- self.class.joins(:route).where('routes.path LIKE ?', "#{route.path}/%").reorder('routes.path ASC')
+ self.class.joins(:route).merge(Route.inside_path(route.path)).reorder('routes.path ASC')
end
def user_ids_for_project_authorizations
diff --git a/app/models/project.rb b/app/models/project.rb
index da4704554b3..04641dd58a0 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -238,7 +238,7 @@ class Project < ActiveRecord::Base
# We need routes alias rs for JOIN so it does not conflict with
# includes(:route) which we use in ProjectsFinder.
joins("INNER JOIN routes rs ON rs.source_id = projects.id AND rs.source_type = 'Project'").
- where('rs.path LIKE ?', "#{path}/%")
+ where('rs.path LIKE ?', "#{sanitize_sql_like(path)}/%")
end
# "enabled" here means "not disabled". It includes private features!
diff --git a/app/models/route.rb b/app/models/route.rb
index 41e6eb7cb73..4b3efab5c3c 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -10,9 +10,11 @@ class Route < ActiveRecord::Base
after_update :rename_descendants
+ scope :inside_path, -> (path) { where('routes.path LIKE ?', "#{sanitize_sql_like(path)}/%") }
+
def rename_descendants
if path_changed? || name_changed?
- descendants = Route.where('path LIKE ?', "#{path_was}/%")
+ descendants = self.class.inside_path(path_was)
descendants.each do |route|
attributes = {}
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 5e5f690acd4..2b5d6f84776 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1762,11 +1762,18 @@ describe Project, models: true do
end
describe 'inside_path' do
- let!(:project1) { create(:empty_project) }
+ let!(:project1) { create(:empty_project, namespace: create(:namespace, path: 'name_pace')) }
let!(:project2) { create(:empty_project) }
+ let!(:project3) { create(:empty_project, namespace: create(:namespace, path: 'namespace')) }
let!(:path) { project1.namespace.full_path }
- it { expect(Project.inside_path(path)).to eq([project1]) }
+ it 'returns 1 project' do
+ expect(Project.inside_path(path).count).to eq(1)
+ end
+
+ it 'returns correct project' do
+ expect(Project.inside_path(path)).to eq([project1])
+ end
end
describe '#route_map_for' do
diff --git a/spec/models/route_spec.rb b/spec/models/route_spec.rb
index bc8ae4ae5a8..05afd9f5b5a 100644
--- a/spec/models/route_spec.rb
+++ b/spec/models/route_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe Route, models: true do
- let!(:group) { create(:group, path: 'gitlab', name: 'gitlab') }
+ let!(:group) { create(:group, path: 'git_lab', name: 'git_lab') }
let!(:route) { group.route }
describe 'relationships' do
@@ -14,10 +14,28 @@ describe Route, models: true do
it { is_expected.to validate_uniqueness_of(:path) }
end
+ describe '.inside_path' do
+ let!(:nested_group) { create(:group, path: 'test', name: 'test', parent: group) }
+ let!(:deep_nested_group) { create(:group, path: 'foo', name: 'foo', parent: nested_group) }
+ let!(:another_group) { create(:group, path: 'other') }
+ let!(:similar_group) { create(:group, path: 'gitllab') }
+ let!(:another_group_nested) { create(:group, path: 'another', name: 'another', parent: similar_group) }
+
+ it 'returns 2 routes' do
+ expect(Route.inside_path('git_lab').count).to eq(2)
+ end
+
+ it 'returns correct routes' do
+ expect(Route.inside_path('git_lab')).to match_array([nested_group.route, deep_nested_group.route])
+ end
+ end
+
describe '#rename_descendants' do
let!(:nested_group) { create(:group, path: 'test', name: 'test', parent: group) }
let!(:deep_nested_group) { create(:group, path: 'foo', name: 'foo', parent: nested_group) }
let!(:similar_group) { create(:group, path: 'gitlab-org', name: 'gitlab-org') }
+ let!(:another_group) { create(:group, path: 'gittlab', name: 'gitllab') }
+ let!(:another_group_nested) { create(:group, path: 'git_lab', name: 'git_lab', parent: another_group) }
context 'path update' do
context 'when route name is set' do
@@ -28,6 +46,8 @@ describe Route, models: true do
expect(described_class.exists?(path: 'bar/test')).to be_truthy
expect(described_class.exists?(path: 'bar/test/foo')).to be_truthy
expect(described_class.exists?(path: 'gitlab-org')).to be_truthy
+ expect(described_class.exists?(path: 'gittlab')).to be_truthy
+ expect(described_class.exists?(path: 'gittlab/git_lab')).to be_truthy
end
end
@@ -44,7 +64,7 @@ describe Route, models: true do
context 'name update' do
it "updates children routes with new path" do
- route.update_attributes(name: 'bar')
+ route.update_attributes(name: 'bar')
expect(described_class.exists?(name: 'bar')).to be_truthy
expect(described_class.exists?(name: 'bar / test')).to be_truthy