From 3be9d2c422b8651498abec3a2ee9bb6a3685f040 Mon Sep 17 00:00:00 2001
From: Ben Ford <ben.ford@puppetlabs.com>
Date: Mon, 19 Oct 2015 14:52:46 -0700
Subject: [PATCH] Add ability to create directories in the editor

Simply type a name with a `/` directory separator and new directories
will be created. This does not do the fancy UI work that github.com
does, but it will get the job done.

I could not find tests for file creation, so I didn't add a test for
this slight behaviour modification. I did test directory traversals
though, using both absolute paths like `/tmp/foo.txt` and relative paths
like `../../foo.txt`. Neither case escaped the repository, though
attempting to traverse with a relative path resulted in a 500 error that
did not affect application stability upon reload.
---
 CHANGELOG                                     |  3 +++
 app/assets/stylesheets/pages/editor.scss      |  2 +-
 app/controllers/projects/blob_controller.rb   |  2 +-
 app/services/files/create_dir_service.rb      | 11 +++++++++++
 app/services/files/create_service.rb          | 11 ++++++++---
 features/project/source/browse_files.feature  | 10 ++++++++++
 features/steps/project/source/browse_files.rb | 15 +++++++++++++++
 lib/gitlab/regex.rb                           | 17 +++++++++++++++++
 8 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 0d89fca9fc1..200e105db4f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,5 +1,8 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+development
+  - Adds ability to create directories using the web editor
+
 v 8.2.0 (unreleased)
   - Improved performance of replacing references in comments
   - Show last project commit to default branch on project home page
diff --git a/app/assets/stylesheets/pages/editor.scss b/app/assets/stylesheets/pages/editor.scss
index 1d565477dd4..e2c521af91e 100644
--- a/app/assets/stylesheets/pages/editor.scss
+++ b/app/assets/stylesheets/pages/editor.scss
@@ -50,7 +50,7 @@
   .editor-file-name {
     .new-file-name {
       display: inline-block;
-      width: 200px;
+      width: 450px;
     }
 
     .form-control {
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 8cc2f21d887..93738aa1ee5 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -161,7 +161,7 @@ class Projects::BlobController < Projects::ApplicationController
         if params[:file].present?
           params[:file_name] = params[:file].original_filename
         end
-        File.join(@path, File.basename(params[:file_name]))
+        File.join(@path, params[:file_name])
       else
         @path
       end
diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb
index 71272fb5707..6107254a34e 100644
--- a/app/services/files/create_dir_service.rb
+++ b/app/services/files/create_dir_service.rb
@@ -5,5 +5,16 @@ module Files
     def commit
       repository.commit_dir(current_user, @file_path, @commit_message, @target_branch)
     end
+
+    def validate
+      super
+
+      unless @file_path =~ Gitlab::Regex.file_path_regex
+        raise_error(
+          'Your changes could not be committed, because the file path ' +
+          Gitlab::Regex.file_path_regex_message
+        )
+      end
+    end
   end
 end
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index c8e3a910bba..2348920cc58 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -9,12 +9,17 @@ module Files
     def validate
       super
 
-      file_name = File.basename(@file_path)
+      if @file_path =~ Gitlab::Regex.directory_traversal_regex
+        raise_error(
+          'Your changes could not be committed, because the file name ' +
+          Gitlab::Regex.directory_traversal_regex_message
+        )
+      end
 
-      unless file_name =~ Gitlab::Regex.file_name_regex
+      unless @file_path =~ Gitlab::Regex.file_path_regex
         raise_error(
           'Your changes could not be committed, because the file name ' +
-          Gitlab::Regex.file_name_regex_message
+          Gitlab::Regex.file_path_regex_message
         )
       end
 
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index 6b0484b6a38..69aa79f2d24 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -90,6 +90,16 @@ Feature: Project Source Browse Files
     Then I am on the new file page
     And I see a commit error message
 
+  @javascript
+  Scenario: I can create file with a directory name
+    Given I click on "New file" link in repo
+    And I fill the new file name with a new directory
+    And I edit code
+    And I fill the commit message
+    And I click on "Commit changes"
+    Then I am redirected to the new file with directory
+    And I should see its new content
+
   @javascript
   Scenario: I can edit file
     Given I click on ".gitignore" file in repo
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 1b27500497a..84725b9b585 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -78,6 +78,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     fill_in :file_name, with: 'Spaces Not Allowed'
   end
 
+  step 'I fill the new file name with a new directory' do
+    fill_in :file_name, with: new_file_name_with_directory
+  end
+
   step 'I fill the commit message' do
     fill_in :commit_message, with: 'Not yet a commit message.', visible: true
   end
@@ -238,6 +242,11 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
       @project.namespace, @project, 'master/' + new_file_name))
   end
 
+  step 'I am redirected to the new file with directory' do
+    expect(current_path).to eq(namespace_project_blob_path(
+      @project.namespace, @project, 'master/' + new_file_name_with_directory))
+  end
+
   step 'I am redirected to the new file on new branch' do
     expect(current_path).to eq(namespace_project_blob_path(
       @project.namespace, @project, 'new_branch_name/' + new_file_name))
@@ -335,6 +344,12 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
     'not_a_file.md'
   end
 
+  # Constant value that is a valid filename with directory and
+  # not a filename present at root of the seed repository.
+  def new_file_name_with_directory
+    'foo/bar/baz.txt'
+  end
+
   # Constant value that is a valid directory and
   # not a directory present at root of the seed repository.
   def new_dir_name
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 9f1adc860d1..53ab2686b43 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -51,6 +51,23 @@ module Gitlab
       "can contain only letters, digits, '_', '-' and '.'. "
     end
 
+    def file_path_regex
+      @file_path_regex ||= /\A[a-zA-Z0-9_\-\.\/]*\z/.freeze
+    end
+
+    def file_path_regex_message
+      "can contain only letters, digits, '_', '-' and '.'. Separate directories with a '/'. "
+    end
+
+
+    def directory_traversal_regex
+      @directory_traversal_regex ||= /\.{2}/.freeze
+    end
+
+    def directory_traversal_regex_message
+      "cannot include directory traversal. "
+    end
+
 
     def archive_formats_regex
       #                           |zip|tar|    tar.gz    |         tar.bz2         |
-- 
GitLab