Merge branch 'fix-path-with-leading-dot-error' into 'master'

Fix error preventing displaying of commit data for a directory with a leading dot

Directories with leading dots erroneously get rejected by the route controller if git
ref regex is used in constraints. This prevents commit data from being loaded. The regex verification is now done in controller.

Closes https://github.com/gitlabhq/gitlabhq/issues/8763


See merge request !1574

6 files changed, 31 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 07ff3d6de42..9633353ed07 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.1.0 (unreleased)
+  - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)
   - Make diff file view easier to use on mobile screens (Stan Hu)
   - Add support for creating directories from Files page (Stan Hu)
   - Allow removing of project without confirmation when JavaScript is disabled (Stan Hu)
diff --git a/app/controllers/projects/refs_controller.rb b/app/controllers/projects/refs_controller.rb
index 6080c849c8d..c4e18c17077 100644
--- a/app/controllers/projects/refs_controller.rb
+++ b/app/controllers/projects/refs_controller.rb
@@ -3,6 +3,7 @@ class Projects::RefsController < Projects::ApplicationController
   include TreeHelper
 
   before_action :require_non_empty_project
+  before_action :validate_ref_id
   before_action :assign_ref_vars
   before_action :authorize_download_code!
 
@@ -71,4 +72,10 @@ class Projects::RefsController < Projects::ApplicationController
       format.js
     end
   end
+
+  private
+
+  def validate_ref_id
+    return not_found! if params[:id].present? && params[:id] !~ Gitlab::Regex.git_reference_regex
+  end
 end
diff --git a/config/routes.rb b/config/routes.rb
index 8e6fbf6340c..893ab59c327 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -543,8 +543,10 @@ Gitlab::Application.routes.draw do
           member do
             # tree viewer logs
             get 'logs_tree', constraints: { id: Gitlab::Regex.git_reference_regex }
+            # Directories with leading dots erroneously get rejected if git
+            # ref regex used in constraints. Regex verification now done in controller.
             get 'logs_tree/*path' => 'refs#logs_tree', as: :logs_file, constraints: {
-              id: Gitlab::Regex.git_reference_regex,
+              id: /.*/,
               path: /.*/
             }
           end
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index 377c5e1a9a7..6b0484b6a38 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -205,3 +205,9 @@ Feature: Project Source Browse Files
     And I see the ref 'test' has been selected
     And I visit the 'test' tree
     Then I see the commit data
+
+  @javascript
+  Scenario: I browse code with a leading dot in the directory
+    Given I switch ref to fix
+    And I visit the fix tree
+    Then I see the commit data for a directory with a leading dot
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index cb100ca0f54..1b27500497a 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -286,6 +286,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     select "'test'", from: 'ref'
   end
 
+  step "I switch ref to fix" do
+    select "fix", from: 'ref'
+  end
+
   step "I see the ref 'test' has been selected" do
     expect(page).to have_selector '.select2-chosen', text: "'test'"
   end
@@ -294,11 +298,20 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     visit namespace_project_tree_path(@project.namespace, @project, "'test'")
   end
 
+  step "I visit the fix tree" do
+    visit namespace_project_tree_path(@project.namespace, @project, "fix/.testdir")
+  end
+
   step 'I see the commit data' do
     expect(page).to have_css('.tree-commit-link', visible: true)
     expect(page).not_to have_content('Loading commit data...')
   end
 
+  step 'I see the commit data for a directory with a leading dot' do
+    expect(page).to have_css('.tree-commit-link', visible: true)
+    expect(page).not_to have_content('Loading commit data...')
+  end
+
   private
 
   def set_new_content
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 3eab74ba986..d12ba25b71b 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -9,7 +9,7 @@ module TestEnv
     'flatten-dir'      => 'e56497b',
     'feature'          => '0b4bc9a',
     'feature_conflict' => 'bb5206f',
-    'fix'              => '12d65c8',
+    'fix'              => '48f0be4',
     'improve/awesome'  => '5937ac0',
     'markdown'         => '0ed8c6c',
     'master'           => '5937ac0',