diff options
| -rw-r--r-- | app/controllers/projects/artifacts_controller.rb | 5 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/artifacts/metadata.rb | 10 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/artifacts/metadata/path.rb | 12 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb | 8 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 8 |
6 files changed, 28 insertions, 19 deletions
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 9f9861dec79..f88d866febc 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -16,7 +16,10 @@ class Projects::ArtifactsController < Projects::ApplicationController def browse return render_404 unless build.artifacts? - @path = build.artifacts_metadata_path(params[:path].to_s) + + directory = params[:path] ? "#{params[:path]}/" : '' + @path = build.artifacts_metadata_path(directory) + return render_404 unless @path.exists? end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 2d2206cc4d7..7593ceda488 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -347,10 +347,8 @@ module Ci artifacts? && artifacts_file.path.end_with?('zip') && artifacts_metadata.exists? end - def artifacts_metadata_path(path) - metadata_file = artifacts_metadata.path - Gitlab::Ci::Build::Artifacts::Metadata.new(metadata_file, path).to_path + Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path).to_path end private diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 996b5d91ff2..91017f633a0 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -12,7 +12,6 @@ module Gitlab def initialize(file, path) @file, @path = file, path @full_version = read_version - @path << '/' unless path.end_with?('/') || path.empty? end def version @@ -43,14 +42,15 @@ module Gitlab def match_entries(gz) paths, metadata = [], [] - child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$} + match_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$} until gz.eof? do begin path = read_string(gz) meta = read_string(gz) - next unless path =~ child_pattern + next unless path =~ match_pattern + next unless path_valid?(path) paths.push(path) metadata.push(JSON.parse(meta.chomp, symbolize_names: true)) @@ -62,6 +62,10 @@ module Gitlab [paths, metadata] end + def path_valid?(path) + !(path.start_with?('/') || path =~ %r{\.?\./}) + end + def read_version gzip do|gz| version_string = read_string(gz) diff --git a/lib/gitlab/ci/build/artifacts/metadata/path.rb b/lib/gitlab/ci/build/artifacts/metadata/path.rb index 222903b348e..80ead335d57 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/path.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/path.rb @@ -23,7 +23,7 @@ module Gitlab end def directory? - @path.end_with?('/') || @path.blank? + blank_node? || @path.end_with?('/') end def file? @@ -40,11 +40,11 @@ module Gitlab end def basename - directory? ? name + ::File::SEPARATOR : name + (directory? && !blank_node?) ? name + ::File::SEPARATOR : name end def name - @name || @path.split(::File::SEPARATOR).last + @name || @path.split(::File::SEPARATOR).last.to_s end def children @@ -83,7 +83,11 @@ module Gitlab end def exists? - @path.blank? || @universe.include?(@path) + blank_node? || @universe.include?(@path) + end + + def blank_node? + @path.empty? # "" is considered to be './' end def to_s diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb index 148d05b5902..3b254c3ce2f 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -108,14 +108,14 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Path do end end - describe '#nodes', path: './test' do + describe '#nodes', path: 'test' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end - describe '#nodes', path: './test/' do + describe '#nodes', path: 'test/' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end describe '#metadata' do diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 36c4851126c..456314768be 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -28,8 +28,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2' do - subject { metadata('other_artifacts_0.1.2').match! } + describe '#match! other_artifacts_0.1.2/' do + subject { metadata('other_artifacts_0.1.2/').match! } it 'matches correct paths' do expect(subject.first). @@ -39,7 +39,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/another-subdirectory' do + describe '#match! other_artifacts_0.1.2/another-subdirectory/' do subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } it 'matches correct paths' do @@ -52,7 +52,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do describe '#to_path' do subject { metadata('').to_path } - it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Path) } end describe '#full_version' do |