From 3ac54d22b940f781c4b870a9d8823a96ad117a53 Mon Sep 17 00:00:00 2001
From: Mark Fletcher <mark@gitlab.com>
Date: Tue, 28 Feb 2017 21:01:55 +0530
Subject: [PATCH] Ensure archive download is only one directory deep

---
 ...feature-xxxx-add-extra-directory-level.yml |  4 ++
 lib/gitlab/git/repository.rb                  |  8 +++-
 spec/lib/gitlab/git/repository_spec.rb        | 44 ++++++++++++-------
 3 files changed, 39 insertions(+), 17 deletions(-)
 create mode 100644 changelogs/unreleased/28805-download-archive-with-branch-like-feature-xxxx-add-extra-directory-level.yml

diff --git a/changelogs/unreleased/28805-download-archive-with-branch-like-feature-xxxx-add-extra-directory-level.yml b/changelogs/unreleased/28805-download-archive-with-branch-like-feature-xxxx-add-extra-directory-level.yml
new file mode 100644
index 00000000000..38ff6b97b2b
--- /dev/null
+++ b/changelogs/unreleased/28805-download-archive-with-branch-like-feature-xxxx-add-extra-directory-level.yml
@@ -0,0 +1,4 @@
+---
+title: Ensure archive download is only one directory deep
+merge_request: 9616
+author:
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 8ec90885231..f9904a39ca4 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -199,13 +199,17 @@ module Gitlab
         nil
       end
 
+      def archive_prefix(ref, sha)
+        project_name = self.name.chomp('.git')
+        "#{project_name}-#{ref.parameterize}-#{sha}"
+      end
+
       def archive_metadata(ref, storage_path, format = "tar.gz")
         ref ||= root_ref
         commit = Gitlab::Git::Commit.find(self, ref)
         return {} if commit.nil?
 
-        project_name = self.name.chomp('.git')
-        prefix = "#{project_name}-#{ref}-#{commit.id}"
+        prefix = archive_prefix(ref, commit.id)
 
         {
           'RepoPath' => path,
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 1919542ca25..3f11f0a4516 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -47,7 +47,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe :branch_names do
+  describe '#branch_names' do
     subject { repository.branch_names }
 
     it 'has SeedRepo::Repo::BRANCHES.size elements' do
@@ -57,7 +57,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.not_to include("branch-from-space") }
   end
 
-  describe :tag_names do
+  describe '#tag_names' do
     subject { repository.tag_names }
 
     it { is_expected.to be_kind_of Array }
@@ -78,49 +78,63 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { expect(metadata['ArchivePath']).to end_with extenstion }
   end
 
-  describe :archive do
+  describe '#archive_prefix' do
+    let(:project_name) { 'project-name'}
+
+    before do
+      expect(repository).to receive(:name).once.and_return(project_name)
+    end
+
+    it 'returns parameterised string for a ref containing slashes' do
+      prefix = repository.archive_prefix('test/branch', 'SHA')
+
+      expect(prefix).to eq("#{project_name}-test-branch-SHA")
+    end
+  end
+
+  describe '#archive' do
     let(:metadata) { repository.archive_metadata('master', '/tmp') }
 
     it_should_behave_like 'archive check', '.tar.gz'
   end
 
-  describe :archive_zip do
+  describe '#archive_zip' do
     let(:metadata) { repository.archive_metadata('master', '/tmp', 'zip') }
 
     it_should_behave_like 'archive check', '.zip'
   end
 
-  describe :archive_bz2 do
+  describe '#archive_bz2' do
     let(:metadata) { repository.archive_metadata('master', '/tmp', 'tbz2') }
 
     it_should_behave_like 'archive check', '.tar.bz2'
   end
 
-  describe :archive_fallback do
+  describe '#archive_fallback' do
     let(:metadata) { repository.archive_metadata('master', '/tmp', 'madeup') }
 
     it_should_behave_like 'archive check', '.tar.gz'
   end
 
-  describe :size do
+  describe '#size' do
     subject { repository.size }
 
     it { is_expected.to be < 2 }
   end
 
-  describe :has_commits? do
+  describe '#has_commits?' do
     it { expect(repository.has_commits?).to be_truthy }
   end
 
-  describe :empty? do
+  describe '#empty?' do
     it { expect(repository.empty?).to be_falsey }
   end
 
-  describe :bare? do
+  describe '#bare?' do
     it { expect(repository.bare?).to be_truthy }
   end
 
-  describe :heads do
+  describe '#heads' do
     let(:heads) { repository.heads }
     subject { heads }
 
@@ -147,7 +161,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe :ref_names do
+  describe '#ref_names' do
     let(:ref_names) { repository.ref_names }
     subject { ref_names }
 
@@ -164,7 +178,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe :search_files do
+  describe '#search_files' do
     let(:results) { repository.search_files('rails', 'master') }
     subject { results }
 
@@ -200,7 +214,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  context :submodules do
+  context '#submodules' do
     let(:repository) { Gitlab::Git::Repository.new(TEST_REPO_PATH) }
 
     context 'where repo has submodules' do
@@ -264,7 +278,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe :commit_count do
+  describe '#commit_count' do
     it { expect(repository.commit_count("master")).to eq(25) }
     it { expect(repository.commit_count("feature")).to eq(9) }
   end