diff options
| -rw-r--r-- | app/controllers/projects/artifacts_controller.rb | 8 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 10 | ||||
| -rw-r--r-- | config/routes.rb | 1 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/artifacts/metadata.rb | 74 | ||||
| -rw-r--r-- | lib/gitlab/ci/build/artifacts/metadata/path.rb | 114 | ||||
| -rw-r--r-- | lib/gitlab/string_path.rb | 139 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb (renamed from spec/lib/gitlab/string_path_spec.rb) | 46 | ||||
| -rw-r--r-- | spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 22 |
8 files changed, 175 insertions, 239 deletions
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 647bcc31de5..9f9861dec79 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -16,10 +16,16 @@ class Projects::ArtifactsController < Projects::ApplicationController def browse return render_404 unless build.artifacts? - @path = build.artifacts_metadata_string_path(params[:path] || './') + @path = build.artifacts_metadata_path(params[:path].to_s) return render_404 unless @path.exists? end + def file + # TODO, check if file exists in metadata + render json: { repository: build.artifacts_file.path, + path: Base64.encode64(params[:path].to_s) } + end + private def build diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 7983ce0e88e..2d2206cc4d7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -338,21 +338,19 @@ module Ci end def artifacts_browse_url - if artifacts? + if artifacts_browser_supported? browse_namespace_project_build_artifacts_path(project.namespace, project, self) end end def artifacts_browser_supported? - # TODO, since carrierwave 0.10.0 we will be able to check mime type here - # artifacts? && artifacts_file.path.end_with?('zip') && artifacts_metadata.exists? end - def artifacts_metadata_string_path(path) - file = artifacts_metadata.path - Gitlab::Ci::Build::Artifacts::Metadata.new(file, path).to_string_path + def artifacts_metadata_path(path) + metadata_file = artifacts_metadata.path + Gitlab::Ci::Build::Artifacts::Metadata.new(metadata_file, path).to_path end private diff --git a/config/routes.rb b/config/routes.rb index 20abf408b73..0ba81a3411a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -611,6 +611,7 @@ Rails.application.routes.draw do resource :artifacts, only: [] do get :download get 'browse(/*path)', action: :browse, as: :browse, format: false + get 'file/*path', action: :file, as: :file, format: false end end diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index d90a64fdbb8..996b5d91ff2 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -6,65 +6,54 @@ module Gitlab module Build module Artifacts class Metadata - def initialize(file, path) - @file = file - - @path = path.sub(/^\.\//, '') - @path << '/' unless path.end_with?('/') - end + VERSION_PATTERN = '[\w\s]+(\d+\.\d+\.\d+)' + attr_reader :file, :path, :full_version - def exists? - File.exists?(@file) - end - - def full_version - gzip do|gz| - read_string(gz) do |size| - raise StandardError, 'Artifacts metadata file empty!' unless size - end - end + def initialize(file, path) + @file, @path = file, path + @full_version = read_version + @path << '/' unless path.end_with?('/') || path.empty? end def version - full_version.match(/\w+ (\d+\.\d+\.\d+)/).captures.first + @full_version.match(/#{VERSION_PATTERN}/).captures.first end def errors gzip do|gz| read_string(gz) # version - JSON.parse(read_string(gz)) + errors = read_string(gz) + raise StandardError, 'Errors field not found!' unless errors + JSON.parse(errors) end end def match! - raise StandardError, 'Metadata file not found !' unless exists? - gzip do |gz| - read_string(gz) # version field - read_string(gz) # errors field - iterate_entries(gz) + 2.times { read_string(gz) } # version and errors fields + match_entries(gz) end end - def to_string_path - universe, metadata = match! - ::Gitlab::StringPath.new(@path, universe, metadata) + def to_path + Path.new(@path, *match!) end private - def iterate_entries(gz) + def match_entries(gz) paths, metadata = [], [] - + child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$} + until gz.eof? do begin path = read_string(gz) meta = read_string(gz) - next unless path =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?$} - + next unless path =~ child_pattern + paths.push(path) - metadata.push(JSON.parse(meta, symbolize_names: true)) + metadata.push(JSON.parse(meta.chomp, symbolize_names: true)) rescue JSON::ParserError next end @@ -73,16 +62,31 @@ module Gitlab [paths, metadata] end - def read_string_size(gz) + def read_version + gzip do|gz| + version_string = read_string(gz) + + unless version_string + raise StandardError, 'Artifacts metadata file empty!' + end + + unless version_string =~ /^#{VERSION_PATTERN}/ + raise StandardError, 'Invalid version!' + end + + version_string.chomp + end + end + + def read_uint32(gz) binary = gz.read(4) binary.unpack('L>')[0] if binary end def read_string(gz) - string_size = read_string_size(gz) - yield string_size if block_given? + string_size = read_uint32(gz) return false unless string_size - gz.read(string_size).chomp + gz.read(string_size) end def gzip diff --git a/lib/gitlab/ci/build/artifacts/metadata/path.rb b/lib/gitlab/ci/build/artifacts/metadata/path.rb new file mode 100644 index 00000000000..222903b348e --- /dev/null +++ b/lib/gitlab/ci/build/artifacts/metadata/path.rb @@ -0,0 +1,114 @@ +module Gitlab + module Ci::Build::Artifacts + class Metadata + ## + # Class that represents a simplified path to a file or + # directory in GitLab CI Build Artifacts binary file / archive + # + # This is IO-operations safe class, that does similar job to + # Ruby's Pathname but without the risk of accessing filesystem. + # + class Path + attr_reader :path, :universe + attr_accessor :name + + def initialize(path, universe, metadata = []) + @path = path + @universe = universe + @metadata = metadata + + if path.include?("\0") + raise ArgumentError, 'Path contains zero byte character!' + end + end + + def directory? + @path.end_with?('/') || @path.blank? + end + + def file? + !directory? + end + + def has_parent? + nodes > 0 + end + + def parent + return nil unless has_parent? + new(@path.chomp(basename)) + end + + def basename + directory? ? name + ::File::SEPARATOR : name + end + + def name + @name || @path.split(::File::SEPARATOR).last + end + + def children + return [] unless directory? + return @children if @children + + child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]+/?$} + @children = select { |entry| entry =~ child_pattern } + end + + def directories + return [] unless directory? + children.select(&:directory?) + end + + def directories! + return directories unless has_parent? + + dotted_parent = parent + dotted_parent.name = '..' + directories.prepend(dotted_parent) + end + + def files + return [] unless directory? + children.select(&:file?) + end + + def metadata + @index ||= @universe.index(@path) + @metadata[@index] || {} + end + + def nodes + @path.count('/') + (file? ? 1 : 0) + end + + def exists? + @path.blank? || @universe.include?(@path) + end + + def to_s + @path + end + + def ==(other) + @path == other.path && @universe == other.universe + end + + def inspect + "#{self.class.name}: #{@path}" + end + + private + + def new(path) + self.class.new(path, @universe, @metadata) + end + + def select + selected = @universe.select { |entry| yield entry } + selected.map { |path| new(path) } + end + end + end + end +end diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb deleted file mode 100644 index 774d4244a2a..00000000000 --- a/lib/gitlab/string_path.rb +++ /dev/null @@ -1,139 +0,0 @@ -module Gitlab - ## - # Class that represents a simplified path to a file or directory - # - # This is IO-operations safe class, that does similar job to - # Ruby's Pathname but without the risk of accessing filesystem. - # - class StringPath - attr_reader :path, :universe - attr_accessor :name - - def initialize(path, universe, metadata = []) - @path = sanitize(path) - @universe = universe.map { |entry| sanitize(entry) } - @metadata = metadata - end - - def to_s - @path - end - - def absolute? - @path.start_with?('/') - end - - def relative? - !absolute? - end - - def directory? - @path.end_with?('/') - end - - def file? - !directory? - end - - def has_parent? - nodes > 1 - end - - def parent - return nil unless has_parent? - new(@path.sub(basename, '')) - end - - def basename - directory? ? name + ::File::SEPARATOR : name - end - - def name - @name || @path.split(::File::SEPARATOR).last - end - - def has_descendants? - descendants.any? - end - - def descendants - return [] unless directory? - select { |entry| entry =~ /^#{Regexp.escape(@path)}.+/ } - end - - def children - return [] unless directory? - return @children if @children - - @children = select do |entry| - entry =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?$} - end - end - - def directories - return [] unless directory? - children.select(&:directory?) - end - - def directories! - return directories unless has_parent? && directory? - - dotted_parent = parent - dotted_parent.name = '..' - directories.prepend(dotted_parent) - end - - def files - return [] unless directory? - children.select(&:file?) - end - - def metadata - index = @universe.index(@path) - @metadata[index] || {} - end - - def nodes - @path.count('/') + (file? ? 1 : 0) - end - - def exists? - @path == './' || @universe.include?(@path) - end - - def ==(other) - @path == other.path && @universe == other.universe - end - - def inspect - "#{self.class.name}: #{@path}" - end - - private - - def new(path) - self.class.new(path, @universe, @metadata) - end - - def select - selected = @universe.select { |entry| yield entry } - selected.map { |path| new(path) } - end - - def sanitize(path) - self.class.sanitize(path) - end - - def self.sanitize(path) - # It looks like Pathname#new doesn't touch a file system, - # neither Pathname#cleanpath does, so it is, hopefully, filesystem safe - - clean_path = Pathname.new(path).cleanpath.to_s - raise ArgumentError, 'Invalid path' if clean_path.start_with?('../') - - prefix = './' unless clean_path =~ %r{^[\.|/]} - suffix = '/' if path.end_with?('/') || ['.', '..'].include?(clean_path) - prefix.to_s + clean_path + suffix.to_s - end - end -end diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb index 7f1d111478b..148d05b5902 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::StringPath do +describe Gitlab::Ci::Build::Artifacts::Metadata::Path do let(:universe) do ['path/', 'path/dir_1/', @@ -27,30 +27,19 @@ describe Gitlab::StringPath do describe '/file/with/absolute_path', path: '/file/with/absolute_path' do subject { |example| path(example) } - it { is_expected.to be_absolute } - it { is_expected.to_not be_relative } it { is_expected.to be_file } it { is_expected.to have_parent } - it { is_expected.to_not have_descendants } - it { is_expected.to exist } describe '#basename' do subject { |example| path(example).basename } - it { is_expected.to eq 'absolute_path' } end end - describe 'path/', path: 'path/' do - subject { |example| path(example) } - - it { is_expected.to be_directory } - it { is_expected.to be_relative } - end - describe 'path/dir_1/', path: 'path/dir_1/' do subject { |example| path(example) } it { is_expected.to have_parent } + it { is_expected.to be_directory } describe '#basename' do subject { |example| path(example).basename } @@ -67,19 +56,6 @@ describe Gitlab::StringPath do it { is_expected.to eq string_path('path/') } end - describe '#descendants' do - subject { |example| path(example).descendants } - - it { is_expected.to be_an_instance_of Array } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/'), - string_path('path/dir_1/subdir/subfile') - end - end - describe '#children' do subject { |example| path(example).children } @@ -117,23 +93,14 @@ describe Gitlab::StringPath do it { is_expected.to all(be_an_instance_of described_class) } it do is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/dir_1/../') + string_path('path/') end end end - describe './', path: './' do + describe 'empty path', path: '' do subject { |example| path(example) } - it { is_expected.to_not have_parent } - it { is_expected.to have_descendants } - - describe '#descendants' do - subject { |example| path(example).descendants } - - it { expect(subject.count).to eq universe.count - 1 } - it { is_expected.to_not include string_path('./') } - end describe '#children' do subject { |example| path(example).children } @@ -141,11 +108,6 @@ describe Gitlab::StringPath do end end - describe '#nodes', path: './' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - describe '#nodes', path: './test' do subject { |example| path(example).nodes } it { is_expected.to eq 2 } diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 0c8a41cfab7..36c4851126c 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -10,13 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file exists' do - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be true } - end - - describe '#match! ./' do - subject { metadata('./').match! } + describe '#match! empty string' do + subject { metadata('').match! } it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', @@ -55,9 +50,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#to_string_path' do - subject { metadata('').to_string_path } - it { is_expected.to be_an_instance_of(Gitlab::StringPath) } + describe '#to_path' do + subject { metadata('').to_path } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } end describe '#full_version' do @@ -79,14 +74,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do context 'metadata file does not exist' do let(:metadata_file_path) { '' } - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be false } - end - describe '#match!' do it 'raises error' do - expect { metadata.match! }.to raise_error(StandardError, /Metadata file not found/) + expect { metadata.match! }.to raise_error(Errno::ENOENT) end end end |