Commit 9fc92fc6 authored by Peter Leitzen's avatar Peter Leitzen

Merge branch 'vij-snippet-repository-exceptions' into 'master'

Refactor snippet repo path error handling

See merge request gitlab-org/gitlab!31224
parents 15136825 b9cb04b0
...@@ -7,6 +7,7 @@ class SnippetRepository < ApplicationRecord ...@@ -7,6 +7,7 @@ class SnippetRepository < ApplicationRecord
EMPTY_FILE_PATTERN = /^#{DEFAULT_EMPTY_FILE_NAME}(\d+)\.txt$/.freeze EMPTY_FILE_PATTERN = /^#{DEFAULT_EMPTY_FILE_NAME}(\d+)\.txt$/.freeze
CommitError = Class.new(StandardError) CommitError = Class.new(StandardError)
InvalidPathError = Class.new(CommitError)
belongs_to :snippet, inverse_of: :snippet_repository belongs_to :snippet, inverse_of: :snippet_repository
...@@ -40,8 +41,9 @@ class SnippetRepository < ApplicationRecord ...@@ -40,8 +41,9 @@ class SnippetRepository < ApplicationRecord
rescue Gitlab::Git::Index::IndexError, rescue Gitlab::Git::Index::IndexError,
Gitlab::Git::CommitError, Gitlab::Git::CommitError,
Gitlab::Git::PreReceiveError, Gitlab::Git::PreReceiveError,
Gitlab::Git::CommandError => e Gitlab::Git::CommandError => error
raise CommitError, e.message
raise commit_error_exception(error)
end end
def transform_file_entries(files) def transform_file_entries(files)
...@@ -85,4 +87,16 @@ class SnippetRepository < ApplicationRecord ...@@ -85,4 +87,16 @@ class SnippetRepository < ApplicationRecord
def build_empty_file_name(index) def build_empty_file_name(index)
"#{DEFAULT_EMPTY_FILE_NAME}#{index}.txt" "#{DEFAULT_EMPTY_FILE_NAME}#{index}.txt"
end end
def commit_error_exception(error)
if error.is_a?(Gitlab::Git::Index::IndexError) && invalid_path_error?(error.message)
InvalidPathError.new('Invalid Path') # To avoid returning the message with the path included
else
CommitError.new(error.message)
end
end
def invalid_path_error?(message)
message.downcase.start_with?('invalid path', 'path cannot include directory traversal')
end
end end
...@@ -117,7 +117,7 @@ module Gitlab ...@@ -117,7 +117,7 @@ module Gitlab
# the migration can succeed, to achieve that, we'll identify in migration retries # the migration can succeed, to achieve that, we'll identify in migration retries
# that the path is invalid # that the path is invalid
def set_file_path_error(error) def set_file_path_error(error)
@invalid_path_error = error.message.downcase.start_with?('invalid path', 'path cannot include directory traversal') @invalid_path_error = error.is_a?(SnippetRepository::InvalidPathError)
end end
end end
end end
......
...@@ -202,6 +202,22 @@ describe SnippetRepository do ...@@ -202,6 +202,22 @@ describe SnippetRepository do
it_behaves_like 'snippet repository with file names', 'snippetfile10.txt', 'snippetfile11.txt' it_behaves_like 'snippet repository with file names', 'snippetfile10.txt', 'snippetfile11.txt'
end end
shared_examples 'snippet repository with git errors' do |path, error|
let(:new_file) { { file_path: path, content: 'bar' } }
it 'raises a path specific error' do
expect do
snippet_repository.multi_files_action(user, data, commit_opts)
end.to raise_error(error)
end
end
context 'with git errors' do
it_behaves_like 'snippet repository with git errors', 'invalid://path/here', described_class::InvalidPathError
it_behaves_like 'snippet repository with git errors', '../../path/traversal/here', described_class::InvalidPathError
it_behaves_like 'snippet repository with git errors', 'README', described_class::CommitError
end
end end
def blob_at(snippet, path) def blob_at(snippet, path)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment