From 79393a351db47afa0df3588b5cdf9fb254c75282 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@gitlab.com>
Date: Fri, 16 Jun 2017 12:11:33 +0200
Subject: [PATCH] Rebuild the dynamic path before validating it

Otherwise we won't validate updates to the path. Allowing users to
change the path to something that's not allowed.
---
 app/models/concerns/routable.rb                | 16 ++++++++--------
 app/validators/dynamic_path_validator.rb       |  2 +-
 spec/validators/dynamic_path_validator_spec.rb |  9 +++++++++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 63d02b76f6b..ec7796a9dbb 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -107,6 +107,14 @@ module Routable
     RequestStore[key] ||= uncached_full_path
   end
 
+  def build_full_path
+    if parent && path
+      parent.full_path + '/' + path
+    else
+      path
+    end
+  end
+
   private
 
   def uncached_full_path
@@ -135,14 +143,6 @@ module Routable
     end
   end
 
-  def build_full_path
-    if parent && path
-      parent.full_path + '/' + path
-    else
-      path
-    end
-  end
-
   def update_route
     prepare_route
     route.save
diff --git a/app/validators/dynamic_path_validator.rb b/app/validators/dynamic_path_validator.rb
index 27ac60637fd..4688aabc2a8 100644
--- a/app/validators/dynamic_path_validator.rb
+++ b/app/validators/dynamic_path_validator.rb
@@ -26,7 +26,7 @@ class DynamicPathValidator < ActiveModel::EachValidator
   end
 
   def path_valid_for_record?(record, value)
-    full_path = record.respond_to?(:full_path) ? record.full_path : value
+    full_path = record.respond_to?(:build_full_path) ? record.build_full_path : value
 
     return true unless full_path
 
diff --git a/spec/validators/dynamic_path_validator_spec.rb b/spec/validators/dynamic_path_validator_spec.rb
index 8dbf3eecd23..8bd5306ff98 100644
--- a/spec/validators/dynamic_path_validator_spec.rb
+++ b/spec/validators/dynamic_path_validator_spec.rb
@@ -84,5 +84,14 @@ describe DynamicPathValidator do
 
       expect(group.errors[:path]).to include('users is a reserved name')
     end
+
+    it 'updating to an invalid path is not allowed' do
+      project = create(:empty_project)
+      project.path = 'update'
+
+      validator.validate_each(project, :path, 'update')
+
+      expect(project.errors[:path]).to include('update is a reserved name')
+    end
   end
 end