From ea4777ff501e370a39ae30e76a955136afe3c1fa Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 15:19:13 +0100 Subject: Add features for list and show details of variables in API --- lib/api/api.rb | 2 ++ lib/api/entities.rb | 4 ++++ lib/api/variables.rb | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 lib/api/variables.rb (limited to 'lib') diff --git a/lib/api/api.rb b/lib/api/api.rb index 7834262d612..a9e1913f0f2 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -54,5 +54,7 @@ module API mount Keys mount Tags mount Triggers + + mount Variables end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 26e7c956e8f..f71d072f269 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -365,5 +365,9 @@ module API class TriggerRequest < Grape::Entity expose :id, :variables end + + class Variable < Grape::Entity + expose :id, :key, :value + end end end diff --git a/lib/api/variables.rb b/lib/api/variables.rb new file mode 100644 index 00000000000..6517150f6f4 --- /dev/null +++ b/lib/api/variables.rb @@ -0,0 +1,43 @@ +module API + # Projects variables API + class Variables < Grape::API + before { authenticate! } + before { authorize_admin_project } + + resource :projects do + # Get project variables + # + # Parameters: + # id (required) - The ID of a project + # page (optional) - The page number for pagination + # per_page (optional) - The value of items per page to show + # Example Request: + # GET /projects/:id/variables + get ':id/variables' do + variables = user_project.variables + present paginate(variables), with: Entities::Variable + end + + # Get specifica bariable of a project + # + # Parameters: + # id (required) - The ID of a project + # variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated + # as ID other ways it's treated as `key` + # Example Reuest: + # GET /projects/:id/variables/:variable_id + get ':id/variables/:variable_id' do + variable_id = params[:variable_id] + variables = user_project.variables + variables = + if variable_id.match(/^\d+$/) + variables.where(id: variable_id.to_i) + else + variables.where(key: variable_id) + end + + present variables.first, with: Entities::Variable + end + end + end +end -- cgit v1.2.1 From a692ce1c079703c4f3947e1d0a29547189e94d0f Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 16:25:49 +0100 Subject: Add update feature for variables API --- lib/api/variables.rb | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index 6517150f6f4..6522ecba70c 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -24,7 +24,7 @@ module API # id (required) - The ID of a project # variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated # as ID other ways it's treated as `key` - # Example Reuest: + # Example Request: # GET /projects/:id/variables/:variable_id get ':id/variables/:variable_id' do variable_id = params[:variable_id] @@ -38,6 +38,25 @@ module API present variables.first, with: Entities::Variable end + + # Update existing variable of a project + # + # Parameters: + # id (required) - The ID of a project + # variable_id (required) - The ID of a variable + # key (optional) - new value for `key` field of variable + # value (optional) - new value for `value` field of variable + # Example Request: + # PUT /projects/:id/variables/:variable_id + put ':id/variables/:variable_id' do + variable = user_project.variables.where(id: params[:variable_id].to_i).first + + variable.key = params[:key] + variable.value = params[:value] + variable.save! + + present variable, with: Entities::Variable + end end end end -- cgit v1.2.1 From 0d014feb1d216e692882976f0d70c3227eaec4ca Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 16:56:03 +0100 Subject: Add delete feature to variables API --- lib/api/variables.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index 6522ecba70c..c70c7cd9d7b 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -57,6 +57,18 @@ module API present variable, with: Entities::Variable end + + # Delete existing variable of a project + # + # Parameters: + # id (required) - The ID of a project + # variable_id (required) - The ID of a variable + # Exanoke Reqyest: + # DELETE /projects/:id/variables/:variable_id + delete ':id/variables/:variable_id' do + variable = user_project.variables.where(id: params[:variable_id].to_i).first + variable.destroy + end end end end -- cgit v1.2.1 From c5177dd5e2171b047a695802c979cf779522ba8a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 17:03:11 +0100 Subject: Add missing 'not_found' checks in variables API --- lib/api/variables.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index c70c7cd9d7b..dac2ba679c7 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -36,6 +36,8 @@ module API variables.where(key: variable_id) end + return not_found!('Variable') if variables.empty? + present variables.first, with: Entities::Variable end @@ -51,6 +53,8 @@ module API put ':id/variables/:variable_id' do variable = user_project.variables.where(id: params[:variable_id].to_i).first + return not_found!('Variable') unless variable + variable.key = params[:key] variable.value = params[:value] variable.save! @@ -67,6 +71,9 @@ module API # DELETE /projects/:id/variables/:variable_id delete ':id/variables/:variable_id' do variable = user_project.variables.where(id: params[:variable_id].to_i).first + + return not_found!('Variable') unless variable + variable.destroy end end -- cgit v1.2.1 From 937567b767e6d7b34dcaa1d9c83fc75464638683 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 22:30:07 +0100 Subject: Add create feature to variables API --- lib/api/variables.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index dac2ba679c7..fc63ac2f56a 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -41,6 +41,24 @@ module API present variables.first, with: Entities::Variable end + # Create a new variable in project + # + # Parameters: + # id (required) - The ID of a project + # key (required) - The key of variable being created + # value (required) - The value of variable being created + # Example Request: + # POST /projects/:id/variables + post ':id/variables' do + required_attributes! [:key, :value] + + variable = user_project.variables.create(key: params[:key], value: params[:value]) + return render_validation_error!(variable) unless variable.valid? + variable.save! + + present variable, with: Entities::Variable + end + # Update existing variable of a project # # Parameters: @@ -75,6 +93,8 @@ module API return not_found!('Variable') unless variable variable.destroy + + present variable, with: Entities::Variable end end end -- cgit v1.2.1 From 16bd4df083135e2e4a263b2e1bdd71b78a875ef7 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 31 Dec 2015 22:59:06 +0100 Subject: Fix a typo in method description --- lib/api/variables.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index fc63ac2f56a..b8bbcb6ce3b 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -18,7 +18,7 @@ module API present paginate(variables), with: Entities::Variable end - # Get specifica bariable of a project + # Get specifica variable of a project # # Parameters: # id (required) - The ID of a project -- cgit v1.2.1 From b60c146267dfa8dc1c170426e1817c6b2a168d1a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 7 Jan 2016 13:49:38 +0100 Subject: Change :variable_id to :key as resource ID in API --- lib/api/variables.rb | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index b8bbcb6ce3b..cc038e5731d 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -22,19 +22,12 @@ module API # # Parameters: # id (required) - The ID of a project - # variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated - # as ID other ways it's treated as `key` + # key (required) - The `key` of variable # Example Request: - # GET /projects/:id/variables/:variable_id - get ':id/variables/:variable_id' do - variable_id = params[:variable_id] - variables = user_project.variables - variables = - if variable_id.match(/^\d+$/) - variables.where(id: variable_id.to_i) - else - variables.where(key: variable_id) - end + # GET /projects/:id/variables/:key + get ':id/variables/:key' do + key = params[:key] + variables = user_project.variables.where(key: key) return not_found!('Variable') if variables.empty? @@ -45,8 +38,8 @@ module API # # Parameters: # id (required) - The ID of a project - # key (required) - The key of variable being created - # value (required) - The value of variable being created + # key (required) - The key of variable + # value (required) - The value of variable # Example Request: # POST /projects/:id/variables post ':id/variables' do @@ -63,17 +56,15 @@ module API # # Parameters: # id (required) - The ID of a project - # variable_id (required) - The ID of a variable - # key (optional) - new value for `key` field of variable - # value (optional) - new value for `value` field of variable + # key (optional) - The `key` of variable + # value (optional) - New value for `value` field of variable # Example Request: - # PUT /projects/:id/variables/:variable_id - put ':id/variables/:variable_id' do - variable = user_project.variables.where(id: params[:variable_id].to_i).first + # PUT /projects/:id/variables/:key + put ':id/variables/:key' do + variable = user_project.variables.where(key: params[:key]).first return not_found!('Variable') unless variable - variable.key = params[:key] variable.value = params[:value] variable.save! @@ -84,11 +75,11 @@ module API # # Parameters: # id (required) - The ID of a project - # variable_id (required) - The ID of a variable + # key (required) - The ID of a variable # Exanoke Reqyest: - # DELETE /projects/:id/variables/:variable_id - delete ':id/variables/:variable_id' do - variable = user_project.variables.where(id: params[:variable_id].to_i).first + # DELETE /projects/:id/variables/:key + delete ':id/variables/:key' do + variable = user_project.variables.where(key: params[:key]).first return not_found!('Variable') unless variable -- cgit v1.2.1 From b60445906849e84ff52ac6a5d7d501bb5a21eb60 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 7 Jan 2016 14:10:49 +0100 Subject: Update ./doc/api --- lib/api/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f71d072f269..db3164d9d9c 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -367,7 +367,7 @@ module API end class Variable < Grape::Entity - expose :id, :key, :value + expose :key, :value end end end -- cgit v1.2.1 From 6e7db8e23e169bcbf0847ece27b9e44e00ae572b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Dec 2015 16:52:02 -0200 Subject: Prevent ldap_blocked users from being blocked/unblocked by the API --- lib/api/users.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/api/users.rb b/lib/api/users.rb index 0d7813428e2..01fd90139b0 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -284,10 +284,12 @@ module API authenticated_as_admin! user = User.find_by(id: params[:id]) - if user + if !user + not_found!('User') + elsif !user.ldap_blocked? user.block else - not_found!('User') + forbidden!('LDAP blocked users cannot be modified by the API') end end @@ -299,10 +301,12 @@ module API authenticated_as_admin! user = User.find_by(id: params[:id]) - if user + if !user + not_found!('User') + elsif !user.ldap_blocked? user.activate else - not_found!('User') + forbidden!('LDAP blocked users cannot be unblocked by the API') end end end -- cgit v1.2.1 From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Dec 2015 20:56:26 -0200 Subject: LDAP synchronization block/unblock new states --- lib/gitlab/ldap/access.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index c438a3d167b..76cb48d7aa6 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -37,15 +37,15 @@ module Gitlab # Block user in GitLab if he/she was blocked in AD if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter) - user.block + user.ldap_block false else - user.activate if user.blocked? && !ldap_config.block_auto_created_users + user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? true end else # Block the user if they no longer exist in LDAP/AD - user.block + user.ldap_block false end rescue -- cgit v1.2.1 From 47e4613f4adc2d6ef4b066a87ec772ef8044bdd5 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 7 Jan 2016 14:01:01 -0200 Subject: Code style fixes and some code simplified --- lib/gitlab/ldap/access.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 76cb48d7aa6..ebd9260ad5d 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -40,7 +40,9 @@ module Gitlab user.ldap_block false else - user.activate if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? + if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? + user.activate + end true end else -- cgit v1.2.1 From ac6a10f3e88c5d2081b8638df63016089517a844 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 12 Jan 2016 12:29:10 -0200 Subject: Codestyle changes --- lib/api/users.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/api/users.rb b/lib/api/users.rb index 01fd90139b0..fd2128bd179 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -303,10 +303,10 @@ module API if !user not_found!('User') - elsif !user.ldap_blocked? - user.activate - else + elsif user.ldap_blocked? forbidden!('LDAP blocked users cannot be unblocked by the API') + else + user.activate end end end -- cgit v1.2.1 From efb3395b4fc0425ebbc2437ad03f0cd5fc851863 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Tue, 12 Jan 2016 19:32:44 +0100 Subject: Remove blank line --- lib/api/api.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/api/api.rb b/lib/api/api.rb index a9e1913f0f2..098dd975840 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -54,7 +54,6 @@ module API mount Keys mount Tags mount Triggers - mount Variables end end -- cgit v1.2.1 From df548285804fdc40ac7c4f36601e87a534792a4a Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 12:47:11 +0100 Subject: Add some fixes after review --- lib/api/variables.rb | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index cc038e5731d..0c3fb5c8a77 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -27,11 +27,11 @@ module API # GET /projects/:id/variables/:key get ':id/variables/:key' do key = params[:key] - variables = user_project.variables.where(key: key) + variable = user_project.variables.find_by(key: key.to_s) - return not_found!('Variable') if variables.empty? + return not_found!('Variable') unless variable - present variables.first, with: Entities::Variable + present variable, with: Entities::Variable end # Create a new variable in project @@ -46,10 +46,12 @@ module API required_attributes! [:key, :value] variable = user_project.variables.create(key: params[:key], value: params[:value]) - return render_validation_error!(variable) unless variable.valid? - variable.save! - present variable, with: Entities::Variable + if variable.valid? + present variable, with: Entities::Variable + else + render_validation_error!(variable) + end end # Update existing variable of a project @@ -61,14 +63,16 @@ module API # Example Request: # PUT /projects/:id/variables/:key put ':id/variables/:key' do - variable = user_project.variables.where(key: params[:key]).first + variable = user_project.variables.find_by(key: params[:key].to_s) return not_found!('Variable') unless variable - variable.value = params[:value] - variable.save! - - present variable, with: Entities::Variable + attrs = attributes_for_keys [:value] + if variable.update(attrs) + present variable, with: Entities::Variable + else + render_validation_error!(variable) + end end # Delete existing variable of a project @@ -79,10 +83,9 @@ module API # Exanoke Reqyest: # DELETE /projects/:id/variables/:key delete ':id/variables/:key' do - variable = user_project.variables.where(key: params[:key]).first + variable = user_project.variables.find_by(key: params[:key].to_s) return not_found!('Variable') unless variable - variable.destroy present variable, with: Entities::Variable -- cgit v1.2.1 From 9e701ccd48ed442124509aeb68fe6788579efdde Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Wed, 13 Jan 2016 18:47:39 +0100 Subject: Fix some typos --- lib/api/variables.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index 0c3fb5c8a77..d9a055f6c92 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -18,7 +18,7 @@ module API present paginate(variables), with: Entities::Variable end - # Get specifica variable of a project + # Get specific variable of a project # # Parameters: # id (required) - The ID of a project @@ -80,7 +80,7 @@ module API # Parameters: # id (required) - The ID of a project # key (required) - The ID of a variable - # Exanoke Reqyest: + # Example Request: # DELETE /projects/:id/variables/:key delete ':id/variables/:key' do variable = user_project.variables.find_by(key: params[:key].to_s) -- cgit v1.2.1 From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 14 Jan 2016 03:31:27 -0200 Subject: fixed LDAP activation on login to use new ldap_blocked state --- lib/gitlab/ldap/access.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index ebd9260ad5d..a659d179b5f 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -40,9 +40,7 @@ module Gitlab user.ldap_block false else - if (user.blocked? && !ldap_config.block_auto_created_users) || user.ldap_blocked? - user.activate - end + user.activate if user.ldap_blocked? true end else -- cgit v1.2.1 From f5d530865875440d69217cf249715bffaa3d11b8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 11:59:10 +0100 Subject: Add implementation of StringPath class `StringPath` class is something similar to Ruby's `Pathname` class, but does not involve any IO operations. `StringPath` objects require passing string representation of path, and array of paths that represents universe to constructor to be intantiated. --- lib/gitlab/string_path.rb | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 lib/gitlab/string_path.rb (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb new file mode 100644 index 00000000000..be65b41dff5 --- /dev/null +++ b/lib/gitlab/string_path.rb @@ -0,0 +1,35 @@ +module Gitlab + ## + # Class that represents a 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 + def initialize(path, universe) + @path = path + @universe = universe + end + + def absolute? + @path.start_with?('/') + end + + def relative? + !absolute? + end + + def directory? + @path.end_with?('/') + end + + def file? + !directory? + end + + def to_s + @path + end + end +end -- cgit v1.2.1 From 73d2c7a553ca239cdce04af793992fd579ad3e4b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 12:32:21 +0100 Subject: Add new methods to StringPath --- lib/gitlab/string_path.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index be65b41dff5..9ccf54bd62f 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -7,11 +7,17 @@ module Gitlab # # class StringPath + attr_reader :path, :universe + def initialize(path, universe) @path = path @universe = universe end + def to_s + @path + end + def absolute? @path.start_with?('/') end @@ -28,8 +34,17 @@ module Gitlab !directory? end - def to_s - @path + def files + raise NotImplementedError + end + + def basename + name = @path.split(::File::SEPARATOR).last + directory? ? name + ::File::SEPARATOR : name + end + + def ==(other) + @path == other.path && @universe == other.universe end end end -- cgit v1.2.1 From 80a71576ba27d84b3406a8b929328359e2edc9da Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 12:55:50 +0100 Subject: Use `Gitlab::StringPath` in CI build artifacts controller --- lib/gitlab/string_path.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 9ccf54bd62f..3aa6200b572 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -34,6 +34,18 @@ module Gitlab !directory? end + def has_parent? + raise NotImplementedError + end + + def parent + raise NotImplementedError + end + + def directories + raise NotImplementedError + end + def files raise NotImplementedError end -- cgit v1.2.1 From 518b206287318006f9b57382a747b1474b6795a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 19 Dec 2015 09:31:52 +0100 Subject: Add `parent` iteration implementation to `StringPath` --- lib/gitlab/string_path.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 3aa6200b572..3add56d2213 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -35,11 +35,12 @@ module Gitlab end def has_parent? - raise NotImplementedError + @universe.include?(@path.sub(basename, '')) end def parent - raise NotImplementedError + return nil unless has_parent? + new(@path.sub(basename, '')) end def directories @@ -58,5 +59,11 @@ module Gitlab def ==(other) @path == other.path && @universe == other.universe end + + private + + def new(path) + self.class.new(path, @universe) + end end end -- cgit v1.2.1 From d382335dcd9285c9355ed04dc12c5314bca3c024 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 10:11:15 +0100 Subject: Add implementation of remaining methods in `StringPath` --- lib/gitlab/string_path.rb | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 3add56d2213..a2dc63db2f2 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -43,12 +43,24 @@ module Gitlab new(@path.sub(basename, '')) end + def descendants + return [] unless directory? + children = @universe.select { |entry| entry =~ /^#{@path}.+/ } + children.map { |path| new(path) } + end + + def children + descendants.select { |descendant| descendant.parent == self } + end + def directories - raise NotImplementedError + return [] unless directory? + children.select { |child| child.directory? } end def files - raise NotImplementedError + return [] unless directory? + children.select { |child| child.file? } end def basename -- cgit v1.2.1 From 37b2c5dd5521f25a7195e82538a0ffc528c3ec6d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 12:08:04 +0100 Subject: Add support for root path for `StringPath` --- lib/gitlab/string_path.rb | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index a2dc63db2f2..d165829132a 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -1,6 +1,6 @@ module Gitlab ## - # Class that represents a path to a file or directory + # 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. @@ -10,8 +10,9 @@ module Gitlab attr_reader :path, :universe def initialize(path, universe) - @path = path - @universe = universe + @path = prepare(path) + @universe = universe.map { |entry| prepare(entry) } + @universe.unshift('./') unless @universe.include?('./') end def to_s @@ -43,6 +44,15 @@ module Gitlab new(@path.sub(basename, '')) end + def basename + name = @path.split(::File::SEPARATOR).last + directory? ? name + ::File::SEPARATOR : name + end + + def has_descendants? + descendants.any? + end + def descendants return [] unless directory? children = @universe.select { |entry| entry =~ /^#{@path}.+/ } @@ -63,11 +73,6 @@ module Gitlab children.select { |child| child.file? } end - def basename - name = @path.split(::File::SEPARATOR).last - directory? ? name + ::File::SEPARATOR : name - end - def ==(other) @path == other.path && @universe == other.universe end @@ -77,5 +82,10 @@ module Gitlab def new(path) self.class.new(path, @universe) end + + def prepare(path) + return path if path =~ %r{^(/|\.|\.\.)} + path.dup.prepend('./') + end end end -- cgit v1.2.1 From b19e958d86f5363057f006c8dbf9a8e8762618b9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 11:05:22 +0100 Subject: Add support for parent directories in `StringPath` This support is not completed though, as parent directory that is first in collection returned by `directories!` is not iterable yet. --- lib/gitlab/string_path.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index d165829132a..493fceb256d 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -5,6 +5,7 @@ module Gitlab # This is IO-operations safe class, that does similar job to # Ruby's Pathname but without the risk of accessing filesystem. # + # TODO: better support for './' and '../' # class StringPath attr_reader :path, :universe @@ -45,10 +46,13 @@ module Gitlab end def basename - name = @path.split(::File::SEPARATOR).last directory? ? name + ::File::SEPARATOR : name end + def name + @path.split(::File::SEPARATOR).last + end + def has_descendants? descendants.any? end @@ -68,6 +72,10 @@ module Gitlab children.select { |child| child.directory? } end + def directories! + has_parent? ? directories.prepend(new(@path + '../')) : directories + end + def files return [] unless directory? children.select { |child| child.file? } -- cgit v1.2.1 From 1cc26e0f0e2ef62fecb688e45bc51c66c8fdf0a7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 13:02:00 +0100 Subject: Improve performance of `StringPath` --- lib/gitlab/string_path.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 493fceb256d..1eb8162f805 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -5,15 +5,15 @@ module Gitlab # This is IO-operations safe class, that does similar job to # Ruby's Pathname but without the risk of accessing filesystem. # - # TODO: better support for './' and '../' + # TODO: better support for '../' and './' # class StringPath attr_reader :path, :universe def initialize(path, universe) @path = prepare(path) - @universe = universe.map { |entry| prepare(entry) } - @universe.unshift('./') unless @universe.include?('./') + @universe = Set.new(universe.map { |entry| prepare(entry) }) + @universe.add('./') end def to_s @@ -64,7 +64,10 @@ module Gitlab end def children - descendants.select { |descendant| descendant.parent == self } + return [] unless directory? + return @children if @children + children = @universe.select { |entry| entry =~ %r{^#{@path}[^/]+/?$} } + @children = children.map { |path| new(path) } end def directories @@ -85,6 +88,10 @@ module Gitlab @path == other.path && @universe == other.universe end + def inspect + "#{self.class.name}: #{@path}" + end + private def new(path) -- cgit v1.2.1 From c177784d5af6b47ae613f922e075a38fc56ad711 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 29 Dec 2015 11:40:57 +0100 Subject: Use short method call in StringPath instead block --- lib/gitlab/string_path.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 1eb8162f805..af80a502bf6 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -72,7 +72,7 @@ module Gitlab def directories return [] unless directory? - children.select { |child| child.directory? } + children.select(&:directory?) end def directories! @@ -81,7 +81,7 @@ module Gitlab def files return [] unless directory? - children.select { |child| child.file? } + children.select(&:file?) end def ==(other) -- cgit v1.2.1 From 447f56036e837fc9a9c2bcaf382d38dc513a9733 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 31 Dec 2015 09:25:59 +0100 Subject: Use metadata stored in artifacats metadata file --- lib/gitlab/string_path.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index af80a502bf6..bfeb0f852f0 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -5,7 +5,7 @@ module Gitlab # This is IO-operations safe class, that does similar job to # Ruby's Pathname but without the risk of accessing filesystem. # - # TODO: better support for '../' and './' + # TODO, better support for '../' and './' # class StringPath attr_reader :path, :universe -- cgit v1.2.1 From 3de8a4620a70c886c815576dc0a30a745cbb8ccb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 31 Dec 2015 12:21:56 +0100 Subject: Parse artifacts metadata stored in JSON format --- lib/gitlab/string_path.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index bfeb0f852f0..ad68a8ff2d6 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -10,10 +10,11 @@ module Gitlab class StringPath attr_reader :path, :universe - def initialize(path, universe) + def initialize(path, universe, metadata = []) @path = prepare(path) - @universe = Set.new(universe.map { |entry| prepare(entry) }) - @universe.add('./') + @universe = universe.map { |entry| prepare(entry) } + @universe << './' unless @universe.include?('./') + @metadata = metadata end def to_s @@ -84,6 +85,11 @@ module Gitlab children.select(&:file?) end + def metadata + index = @universe.index(@path) + @metadata[index] + end + def ==(other) @path == other.path && @universe == other.universe end -- cgit v1.2.1 From a3191463b60c8ded25a2898d5e5520ae4aff1114 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 2 Jan 2016 09:43:10 +0100 Subject: Add path sanitization to `StringPath` [ci skip] --- lib/gitlab/string_path.rb | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index ad68a8ff2d6..8310564646e 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -5,15 +5,12 @@ module Gitlab # This is IO-operations safe class, that does similar job to # Ruby's Pathname but without the risk of accessing filesystem. # - # TODO, better support for '../' and './' - # class StringPath attr_reader :path, :universe def initialize(path, universe, metadata = []) - @path = prepare(path) - @universe = universe.map { |entry| prepare(entry) } - @universe << './' unless @universe.include?('./') + @path = sanitize(path) + @universe = universe.map { |entry| sanitize(entry) } @metadata = metadata end @@ -60,15 +57,16 @@ module Gitlab def descendants return [] unless directory? - children = @universe.select { |entry| entry =~ /^#{@path}.+/ } - children.map { |path| new(path) } + select { |entry| entry =~ /^#{@path}.+/ } end def children return [] unless directory? return @children if @children - children = @universe.select { |entry| entry =~ %r{^#{@path}[^/]+/?$} } - @children = children.map { |path| new(path) } + + @children = select do |entry| + self.class.child?(@path, entry) + end end def directories @@ -104,9 +102,26 @@ module Gitlab self.class.new(path, @universe) end - def prepare(path) - return path if path =~ %r{^(/|\.|\.\.)} - path.dup.prepend('./') + 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 = Pathname.new(path).cleanpath.to_s + raise ArgumentError, 'Invalid path' if clean.start_with?('../') + clean + (path.end_with?('/') ? '/' : '') + end + + def self.child?(path, entry) + entry =~ %r{^#{path}[^/\s]+/?$} end end end -- cgit v1.2.1 From df41148662142ce20a77b092665f48dd4dfa7bfb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 2 Jan 2016 20:09:21 +0100 Subject: Improve path sanitization in `StringPath` --- lib/gitlab/string_path.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 8310564646e..e7d99a35869 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -57,7 +57,7 @@ module Gitlab def descendants return [] unless directory? - select { |entry| entry =~ /^#{@path}.+/ } + select { |entry| entry =~ /^#{Regexp.escape(@path)}.+/ } end def children @@ -65,7 +65,7 @@ module Gitlab return @children if @children @children = select do |entry| - self.class.child?(@path, entry) + entry =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?$} end end @@ -75,7 +75,7 @@ module Gitlab end def directories! - has_parent? ? directories.prepend(new(@path + '../')) : directories + has_parent? ? directories.prepend(parent) : directories end def files @@ -115,13 +115,12 @@ module Gitlab # It looks like Pathname#new doesn't touch a file system, # neither Pathname#cleanpath does, so it is, hopefully, filesystem safe - clean = Pathname.new(path).cleanpath.to_s - raise ArgumentError, 'Invalid path' if clean.start_with?('../') - clean + (path.end_with?('/') ? '/' : '') - end + clean_path = Pathname.new(path).cleanpath.to_s + raise ArgumentError, 'Invalid path' if clean_path.start_with?('../') - def self.child?(path, entry) - entry =~ %r{^#{path}[^/\s]+/?$} + prefix = './' unless clean_path =~ %r{^[\.|/]} + suffix = '/' if path.end_with?('/') || clean_path =~ /^[\.|\.\.]$/ + prefix.to_s + clean_path + suffix.to_s end end end -- cgit v1.2.1 From a7f99b67a0bf1160f41ebf4dc92c618eb13a7a10 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:08:49 +0100 Subject: Extract artifacts metadata implementation to separate class --- lib/gitlab/ci/build/artifacts/metadata.rb | 57 +++++++++++++++++++++++++++++++ lib/gitlab/string_path.rb | 4 +-- 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 lib/gitlab/ci/build/artifacts/metadata.rb (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb new file mode 100644 index 00000000000..5313182d55f --- /dev/null +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -0,0 +1,57 @@ +require 'zlib' +require 'json' + +module Gitlab + module Ci + module Build + module Artifacts + class Metadata + def initialize(file, path) + @file = file + + @path = path.sub(/^\.\//, '') + @path << '/' unless path.end_with?('/') + end + + def exists? + File.exists?(@file) + end + + def match! + raise StandardError, 'Metadata file not found !' unless exists? + paths, metadata = [], [] + + each do |line| + next unless line =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?\s} + + path, meta = line.split(' ') + paths.push(path) + metadata.push(meta) + end + + [paths, metadata.map { |meta| JSON.parse(meta) }] + end + + def to_string_path + universe, metadata = match! + ::Gitlab::StringPath.new(@path, universe, metadata) + end + + private + + def each + open do |file| + gzip = Zlib::GzipReader.new(file) + gzip.each_line { |line| yield line } + gzip.close + end + end + + def open + File.open(@file) { |file| yield file } + end + end + end + end + end +end diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index e7d99a35869..9948502e8ea 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -75,7 +75,7 @@ module Gitlab end def directories! - has_parent? ? directories.prepend(parent) : directories + @path =~ %r{^\./[^/]/} ? directories.prepend(parent) : directories end def files @@ -119,7 +119,7 @@ module Gitlab raise ArgumentError, 'Invalid path' if clean_path.start_with?('../') prefix = './' unless clean_path =~ %r{^[\.|/]} - suffix = '/' if path.end_with?('/') || clean_path =~ /^[\.|\.\.]$/ + suffix = '/' if path.end_with?('/') || ['.', '..'].include?(clean_path) prefix.to_s + clean_path + suffix.to_s end end -- cgit v1.2.1 From f948c00757ca9529817c7368610b0c0d6734d48f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:40:42 +0100 Subject: Do not depend on universe when checking parent in `StringPath` --- lib/gitlab/string_path.rb | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 9948502e8ea..4d024b3ff73 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -7,6 +7,7 @@ module Gitlab # class StringPath attr_reader :path, :universe + attr_accessor :name def initialize(path, universe, metadata = []) @path = sanitize(path) @@ -35,7 +36,7 @@ module Gitlab end def has_parent? - @universe.include?(@path.sub(basename, '')) + nodes > 1 end def parent @@ -48,7 +49,7 @@ module Gitlab end def name - @path.split(::File::SEPARATOR).last + @name || @path.split(::File::SEPARATOR).last end def has_descendants? @@ -75,7 +76,11 @@ module Gitlab end def directories! - @path =~ %r{^\./[^/]/} ? directories.prepend(parent) : directories + return directories unless has_parent? && directory? + + dotted_parent = parent + dotted_parent.name = '..' + directories.prepend(dotted_parent) end def files @@ -88,6 +93,10 @@ module Gitlab @metadata[index] end + def nodes + @path.count('/') + (file? ? 1 : 0) + end + def ==(other) @path == other.path && @universe == other.universe end -- cgit v1.2.1 From a5e1905d28e490fb4734bff0e02a1ecff4c7c029 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:00:49 +0100 Subject: Render 404 when artifacts path is invalid --- lib/gitlab/ci/build/artifacts/metadata.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 5313182d55f..d5c3cd10e48 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -22,7 +22,7 @@ module Gitlab paths, metadata = [], [] each do |line| - next unless line =~ %r{^#{Regexp.escape(@path)}[^/\s]+/?\s} + next unless line =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?\s} path, meta = line.split(' ') paths.push(path) -- cgit v1.2.1 From cd3b8bbd2f8e7ad75a453441f83c46aeb1d37353 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:18:06 +0100 Subject: Add method that checks if path exists in `StringPath` --- lib/gitlab/string_path.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index 4d024b3ff73..a6234d34e7d 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -19,6 +19,10 @@ module Gitlab @path end + def exists? + @path == './' || @universe.include?(@path) + end + def absolute? @path.start_with?('/') end -- cgit v1.2.1 From 1b1793c2530d7003d8baa5aa1912a4ab258d4a1c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 15:07:49 +0100 Subject: Show file size in artifacts browser using metadata --- lib/gitlab/ci/build/artifacts/metadata.rb | 5 ++--- lib/gitlab/string_path.rb | 12 ++++++------ 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index d5c3cd10e48..1f3000e7c8a 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -23,13 +23,12 @@ module Gitlab each do |line| next unless line =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?\s} - path, meta = line.split(' ') paths.push(path) metadata.push(meta) - end + end - [paths, metadata.map { |meta| JSON.parse(meta) }] + [paths, metadata.map { |meta| JSON.parse(meta, symbolize_names: true) }] end def to_string_path diff --git a/lib/gitlab/string_path.rb b/lib/gitlab/string_path.rb index a6234d34e7d..774d4244a2a 100644 --- a/lib/gitlab/string_path.rb +++ b/lib/gitlab/string_path.rb @@ -19,10 +19,6 @@ module Gitlab @path end - def exists? - @path == './' || @universe.include?(@path) - end - def absolute? @path.start_with?('/') end @@ -94,13 +90,17 @@ module Gitlab def metadata index = @universe.index(@path) - @metadata[index] + @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 @@ -112,7 +112,7 @@ module Gitlab private def new(path) - self.class.new(path, @universe) + self.class.new(path, @universe, @metadata) end def select -- cgit v1.2.1 From 387b27813d1d496c015f4f174812b4761c32648d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 8 Jan 2016 12:35:49 +0100 Subject: Change format of artifacts metadata from text to binary 0.0.1 This changes the format of metadata to handle paths, that may contain whitespace characters, new line characters and non-UTF-8 characters. Now those paths along with metadata in JSON format are stored as length-prefixed strings (uint32 prefix). Metadata file has a custom format: 1. First string field is metadata version field (string) 2. Second string field is metadata errors field (JSON strong) 3. All subsequent fields is pair of path (string) and path metadata in JSON format. Path's metadata contains all fields that where possible to extract from ZIP archive like date of modification, CRC, compressed size, uncompressed size and comment. --- lib/gitlab/ci/build/artifacts/metadata.rb | 68 ++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 1f3000e7c8a..d90a64fdbb8 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -17,18 +17,33 @@ module Gitlab 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 + end + + def version + full_version.match(/\w+ (\d+\.\d+\.\d+)/).captures.first + end + + def errors + gzip do|gz| + read_string(gz) # version + JSON.parse(read_string(gz)) + end + end + def match! raise StandardError, 'Metadata file not found !' unless exists? - paths, metadata = [], [] - each do |line| - next unless line =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?\s} - path, meta = line.split(' ') - paths.push(path) - metadata.push(meta) + gzip do |gz| + read_string(gz) # version field + read_string(gz) # errors field + iterate_entries(gz) end - - [paths, metadata.map { |meta| JSON.parse(meta, symbolize_names: true) }] end def to_string_path @@ -38,11 +53,44 @@ module Gitlab private - def each + def iterate_entries(gz) + paths, metadata = [], [] + + until gz.eof? do + begin + path = read_string(gz) + meta = read_string(gz) + + next unless path =~ %r{^#{Regexp.escape(@path)}[^/\s]*/?$} + + paths.push(path) + metadata.push(JSON.parse(meta, symbolize_names: true)) + rescue JSON::ParserError + next + end + end + + [paths, metadata] + end + + def read_string_size(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? + return false unless string_size + gz.read(string_size).chomp + end + + def gzip open do |file| gzip = Zlib::GzipReader.new(file) - gzip.each_line { |line| yield line } + result = yield gzip gzip.close + result end end -- cgit v1.2.1 From 61fb47a43202332fe9ac57847996da929ba42d3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 9 Jan 2016 14:41:43 +0100 Subject: Simplify implementation of build artifacts browser (refactoring) --- lib/gitlab/ci/build/artifacts/metadata.rb | 74 ++++++------- lib/gitlab/ci/build/artifacts/metadata/path.rb | 114 ++++++++++++++++++++ lib/gitlab/string_path.rb | 139 ------------------------- 3 files changed, 153 insertions(+), 174 deletions(-) create mode 100644 lib/gitlab/ci/build/artifacts/metadata/path.rb delete mode 100644 lib/gitlab/string_path.rb (limited to 'lib') 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 -- cgit v1.2.1 From 09a4a5aff8c53dd5930044ddbb285a95ef177d8a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 11 Jan 2016 09:57:03 +0100 Subject: Render only valid paths in artifacts metadata In this version we will support only relative paths in artifacts metadata. Support for absolute paths will be introduced later. --- lib/gitlab/ci/build/artifacts/metadata.rb | 10 +++++++--- lib/gitlab/ci/build/artifacts/metadata/path.rb | 12 ++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) (limited to 'lib') 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 -- cgit v1.2.1 From ffee05c242c87e004054b48747287c3160d1c19a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 11 Jan 2016 12:50:21 +0100 Subject: Improve invalid build artifacts metadata path matcher --- lib/gitlab/ci/build/artifacts/metadata.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 91017f633a0..2b17712cdbe 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -43,6 +43,7 @@ module Gitlab def match_entries(gz) paths, metadata = [], [] match_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$} + invalid_pattern = %r{(^\.?\.?/)|(/\.?\.?/)} until gz.eof? do begin @@ -50,7 +51,7 @@ module Gitlab meta = read_string(gz) next unless path =~ match_pattern - next unless path_valid?(path) + next if path =~ invalid_pattern paths.push(path) metadata.push(JSON.parse(meta.chomp, symbolize_names: true)) @@ -62,10 +63,6 @@ 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) -- cgit v1.2.1 From 2be76355caa579d444c8e3c0d25563eb9778bfb2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 10:04:26 +0100 Subject: Support only valid UTF-8 paths in build artifacts browser --- lib/gitlab/ci/build/artifacts/metadata.rb | 8 +++++--- lib/gitlab/ci/build/artifacts/metadata/path.rb | 10 ++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 2b17712cdbe..d9c051be9f3 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -10,7 +10,8 @@ module Gitlab attr_reader :file, :path, :full_version def initialize(file, path) - @file, @path = file, path + @file = file + @path = path.force_encoding('ASCII-8BIT') @full_version = read_version end @@ -42,7 +43,7 @@ module Gitlab def match_entries(gz) paths, metadata = [], [] - match_pattern = %r{^#{Regexp.escape(@path)}[^/\s]*/?$} + match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$} invalid_pattern = %r{(^\.?\.?/)|(/\.?\.?/)} until gz.eof? do @@ -51,11 +52,12 @@ module Gitlab meta = read_string(gz) next unless path =~ match_pattern + next unless path.force_encoding('UTF-8').valid_encoding? next if path =~ invalid_pattern paths.push(path) metadata.push(JSON.parse(meta.chomp, symbolize_names: true)) - rescue JSON::ParserError + rescue JSON::ParserError, Encoding::CompatibilityError next end end diff --git a/lib/gitlab/ci/build/artifacts/metadata/path.rb b/lib/gitlab/ci/build/artifacts/metadata/path.rb index 80ead335d57..6896aa936d5 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/path.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/path.rb @@ -8,18 +8,24 @@ module Gitlab # This is IO-operations safe class, that does similar job to # Ruby's Pathname but without the risk of accessing filesystem. # + # This class is working only with UTF-8 encoded paths. + # class Path attr_reader :path, :universe attr_accessor :name def initialize(path, universe, metadata = []) - @path = path + @path = path.force_encoding('UTF-8') @universe = universe @metadata = metadata if path.include?("\0") raise ArgumentError, 'Path contains zero byte character!' end + + unless path.valid_encoding? + raise ArgumentError, 'Path contains non-UTF-8 byte sequence!' + end end def directory? @@ -51,7 +57,7 @@ module Gitlab return [] unless directory? return @children if @children - child_pattern = %r{^#{Regexp.escape(@path)}[^/\s]+/?$} + child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$} @children = select { |entry| entry =~ child_pattern } end -- cgit v1.2.1 From 487b0a026f9efe2d8214c19a7b95b391708ba3f4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 11:02:15 +0100 Subject: Improvements, readability for artifacts browser --- lib/gitlab/ci/build/artifacts/metadata.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index d9c051be9f3..47efc51a76e 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -56,7 +56,7 @@ module Gitlab next if path =~ invalid_pattern paths.push(path) - metadata.push(JSON.parse(meta.chomp, symbolize_names: true)) + metadata.push(JSON.parse(meta, symbolize_names: true)) rescue JSON::ParserError, Encoding::CompatibilityError next end @@ -66,7 +66,7 @@ module Gitlab end def read_version - gzip do|gz| + gzip do |gz| version_string = read_string(gz) unless version_string @@ -95,9 +95,11 @@ module Gitlab def gzip open do |file| gzip = Zlib::GzipReader.new(file) - result = yield gzip - gzip.close - result + begin + yield gzip + ensure + gzip.close + end end end -- cgit v1.2.1 From e8995f9fd5c12882eafcf3766627f64a3d6f623d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 14:39:15 +0100 Subject: Modify artifacts upload API endpoint, add artifacts metadata --- lib/ci/api/builds.rb | 20 ++++++++++++++++---- lib/ci/api/entities.rb | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 15faa6edd84..2aaa06a022c 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -78,11 +78,15 @@ module Ci # Parameters: # id (required) - The ID of a build # token (required) - The build authorization token - # file (required) - The uploaded file + # file (required) - Artifacts file + # metadata (optional) - Artifacts metadata file # Parameters (accelerated by GitLab Workhorse): # file.path - path to locally stored body (generated by Workhorse) # file.name - real filename as send in Content-Disposition # file.type - real content type as send in Content-Type + # metadata.path - path to locally stored body (generated by Workhorse) + # metadata.name - real filename as send in Content-Disposition + # metadata.type - real content type as send in Content-Type # Headers: # BUILD-TOKEN (required) - The build authorization token, the same as token # Body: @@ -98,10 +102,17 @@ module Ci authenticate_build_token!(build) forbidden!('build is not running') unless build.running? - file = uploaded_file!(:file, ArtifactUploader.artifacts_upload_path) - file_to_large! unless file.size < max_artifacts_size + artifacts_upload_path = ArtifactUploader.artifacts_upload_path + artifacts = uploaded_file!(:file, artifacts_upload_path) + file_to_large! unless artifacts.size < max_artifacts_size + artifacts_attributes = { artifacts_file: artifacts } - if build.update_attributes(artifacts_file: file) + if params[:metadata] || params['metadata.path'.to_sym] + metadata = uploaded_file!(:metadata, artifacts_upload_path) + artifacts_attributes.store(:artifacts_metadata, metadata) + end + + if build.update_attributes(artifacts_attributes) present build, with: Entities::Build else render_validation_error!(build) @@ -148,6 +159,7 @@ module Ci not_found! unless build authenticate_build_token!(build) build.remove_artifacts_file! + build.remove_artifacts_metadata! end end end diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index e4ac0545ea2..dd34c661e25 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -31,6 +31,7 @@ module Ci expose :variables expose :artifacts_file, using: ArtifactFile + expose :artifacts_metadata, using: ArtifactFile end class Runner < Grape::Entity -- cgit v1.2.1 From 0b946029a1fb429db39fbec0cddccf40f7e2aa08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 09:56:05 +0100 Subject: Update build artifacts API We do not want to allow runners to upload a metadata file. This needs to be generated by Workhorse only. --- lib/ci/api/builds.rb | 17 ++++++++--------- lib/ci/api/entities.rb | 1 - 2 files changed, 8 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 2aaa06a022c..be2790571cb 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -79,14 +79,13 @@ module Ci # id (required) - The ID of a build # token (required) - The build authorization token # file (required) - Artifacts file - # metadata (optional) - Artifacts metadata file # Parameters (accelerated by GitLab Workhorse): # file.path - path to locally stored body (generated by Workhorse) # file.name - real filename as send in Content-Disposition # file.type - real content type as send in Content-Type # metadata.path - path to locally stored body (generated by Workhorse) - # metadata.name - real filename as send in Content-Disposition - # metadata.type - real content type as send in Content-Type + # metadata.name - filename (generated by Workhorse) + # metadata.type - content type (returned by Workhorse) # Headers: # BUILD-TOKEN (required) - The build authorization token, the same as token # Body: @@ -101,19 +100,19 @@ module Ci not_found! unless build authenticate_build_token!(build) forbidden!('build is not running') unless build.running? + forbidden!('metadata reserved for workhorse') if params[:metadata] artifacts_upload_path = ArtifactUploader.artifacts_upload_path artifacts = uploaded_file!(:file, artifacts_upload_path) file_to_large! unless artifacts.size < max_artifacts_size - artifacts_attributes = { artifacts_file: artifacts } + build.artifacts_file = artifacts - if params[:metadata] || params['metadata.path'.to_sym] - metadata = uploaded_file!(:metadata, artifacts_upload_path) - artifacts_attributes.store(:artifacts_metadata, metadata) + if params[:'metadata.path'] && params[:'metadata.name'] + build.artifacts_metadata = uploaded_file!(:metadata, artifacts_upload_path) end - if build.update_attributes(artifacts_attributes) - present build, with: Entities::Build + if build.save + present(build, with: Entities::Build) else render_validation_error!(build) end diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index dd34c661e25..e4ac0545ea2 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -31,7 +31,6 @@ module Ci expose :variables expose :artifacts_file, using: ArtifactFile - expose :artifacts_metadata, using: ArtifactFile end class Runner < Grape::Entity -- cgit v1.2.1 From a9783c439bd6e3322b6fc72371c9fe3837a63be5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 10:13:39 +0100 Subject: Make encoding of paths returned by metadata consistent (UTF-8) --- lib/gitlab/ci/build/artifacts/metadata.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 47efc51a76e..0c252c0bf30 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -20,7 +20,7 @@ module Gitlab end def errors - gzip do|gz| + gzip do |gz| read_string(gz) # version errors = read_string(gz) raise StandardError, 'Errors field not found!' unless errors @@ -36,7 +36,7 @@ module Gitlab end def to_path - Path.new(@path, *match!) + Path.new(@path.dup.force_encoding('UTF-8'), *match!) end private @@ -88,7 +88,7 @@ module Gitlab def read_string(gz) string_size = read_uint32(gz) - return false unless string_size + return nil unless string_size gz.read(string_size) end -- cgit v1.2.1 From 3f0c18f80e36561581ef6fa6dbfcec169e1a6e08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 13:45:28 +0100 Subject: Simplify encoding related implementation in artifacts metadata --- lib/gitlab/ci/build/artifacts/metadata.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 0c252c0bf30..e9ec8f1302c 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -10,8 +10,7 @@ module Gitlab attr_reader :file, :path, :full_version def initialize(file, path) - @file = file - @path = path.force_encoding('ASCII-8BIT') + @file, @path = file, path @full_version = read_version end @@ -36,7 +35,7 @@ module Gitlab end def to_path - Path.new(@path.dup.force_encoding('UTF-8'), *match!) + Path.new(@path, *match!) end private @@ -48,11 +47,11 @@ module Gitlab until gz.eof? do begin - path = read_string(gz) - meta = read_string(gz) + path = read_string(gz).force_encoding('UTF-8') + meta = read_string(gz).force_encoding('UTF-8') + next unless path.valid_encoding? && meta.valid_encoding? next unless path =~ match_pattern - next unless path.force_encoding('UTF-8').valid_encoding? next if path =~ invalid_pattern paths.push(path) -- cgit v1.2.1 From 154b8ceba4ac2d92a2387ad50d7f2b4ed5b2dd8a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 14:02:36 +0100 Subject: Refactor build artifacts upload API endpoint --- lib/api/helpers.rb | 4 +++- lib/ci/api/builds.rb | 15 +++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a4df810e755..d46b5c42967 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -289,12 +289,14 @@ module API # file helpers - def uploaded_file!(field, uploads_path) + def uploaded_file(field, uploads_path) if params[field] bad_request!("#{field} is not a file") unless params[field].respond_to?(:filename) return params[field] end + return nil unless params["#{field}.path"] && params["#{field}.name"] + # sanitize file paths # this requires all paths to exist required_attributes! %W(#{field}.path) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index be2790571cb..fb87637b94f 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -85,7 +85,6 @@ module Ci # file.type - real content type as send in Content-Type # metadata.path - path to locally stored body (generated by Workhorse) # metadata.name - filename (generated by Workhorse) - # metadata.type - content type (returned by Workhorse) # Headers: # BUILD-TOKEN (required) - The build authorization token, the same as token # Body: @@ -99,17 +98,17 @@ module Ci build = Ci::Build.find_by_id(params[:id]) not_found! unless build authenticate_build_token!(build) - forbidden!('build is not running') unless build.running? - forbidden!('metadata reserved for workhorse') if params[:metadata] + forbidden!('Build is not running!') unless build.running? artifacts_upload_path = ArtifactUploader.artifacts_upload_path - artifacts = uploaded_file!(:file, artifacts_upload_path) + artifacts = uploaded_file(:file, artifacts_upload_path) + metadata = uploaded_file(:metadata, artifacts_upload_path) + + bad_request!('Missing artifacts file!') unless artifacts file_to_large! unless artifacts.size < max_artifacts_size - build.artifacts_file = artifacts - if params[:'metadata.path'] && params[:'metadata.name'] - build.artifacts_metadata = uploaded_file!(:metadata, artifacts_upload_path) - end + build.artifacts_file = artifacts + build.artifacts_metadata = metadata if build.save present(build, with: Entities::Build) -- cgit v1.2.1 From 6b0a43aff36f0bbb9050b3c04155a3ccd9c1a75b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 21:17:28 +0100 Subject: Improve readability of artifacts browser `Entry` related code --- lib/gitlab/ci/build/artifacts/metadata.rb | 5 +- lib/gitlab/ci/build/artifacts/metadata/entry.rb | 125 ++++++++++++++++++++++++ lib/gitlab/ci/build/artifacts/metadata/path.rb | 124 ----------------------- 3 files changed, 128 insertions(+), 126 deletions(-) create mode 100644 lib/gitlab/ci/build/artifacts/metadata/entry.rb delete mode 100644 lib/gitlab/ci/build/artifacts/metadata/path.rb (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index e9ec8f1302c..bfdfc9a1d7d 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -34,8 +34,9 @@ module Gitlab end end - def to_path - Path.new(@path, *match!) + def to_entry + entires, metadata = match! + Entry.new(@path, entires, metadata) end private diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb new file mode 100644 index 00000000000..2fb6c327729 --- /dev/null +++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb @@ -0,0 +1,125 @@ +module Gitlab + module Ci::Build::Artifacts + class Metadata + ## + # Class that represents an entry (path and metadata) 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. + # + # This class is working only with UTF-8 encoded paths. + # + class Entry + attr_reader :path, :entires + attr_accessor :name + + def initialize(path, entires, metadata = []) + @path = path.force_encoding('UTF-8') + @entires = entires + @metadata = metadata + + if path.include?("\0") + raise ArgumentError, 'Path contains zero byte character!' + end + + unless path.valid_encoding? + raise ArgumentError, 'Path contains non-UTF-8 byte sequence!' + end + end + + def directory? + blank_node? || @path.end_with?('/') + 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? && !blank_node?) ? name + '/' : name + end + + def name + @name || @path.split('/').last.to_s + end + + def children + return [] unless directory? + return @children if @children + + child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$} + @children = select_entires { |entry| entry =~ child_pattern } + end + + def directories(opts = {}) + return [] unless directory? + dirs = children.select(&:directory?) + return dirs unless has_parent? && opts[:parent] + + dotted_parent = parent + dotted_parent.name = '..' + dirs.prepend(dotted_parent) + end + + def files + return [] unless directory? + children.select(&:file?) + end + + def metadata + @index ||= @entires.index(@path) + @metadata[@index] || {} + end + + def nodes + @path.count('/') + (file? ? 1 : 0) + end + + def blank_node? + @path.empty? # "" is considered to be './' + end + + def exists? + blank_node? || @entires.include?(@path) + end + + def empty? + children.empty? + end + + def to_s + @path + end + + def ==(other) + @path == other.path && @entires == other.entires + end + + def inspect + "#{self.class.name}: #{@path}" + end + + private + + def new(path) + self.class.new(path, @entires, @metadata) + end + + def select_entires + selected = @entires.select { |entry| yield entry } + selected.map { |path| new(path) } + end + end + end + end +end diff --git a/lib/gitlab/ci/build/artifacts/metadata/path.rb b/lib/gitlab/ci/build/artifacts/metadata/path.rb deleted file mode 100644 index 6896aa936d5..00000000000 --- a/lib/gitlab/ci/build/artifacts/metadata/path.rb +++ /dev/null @@ -1,124 +0,0 @@ -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. - # - # This class is working only with UTF-8 encoded paths. - # - class Path - attr_reader :path, :universe - attr_accessor :name - - def initialize(path, universe, metadata = []) - @path = path.force_encoding('UTF-8') - @universe = universe - @metadata = metadata - - if path.include?("\0") - raise ArgumentError, 'Path contains zero byte character!' - end - - unless path.valid_encoding? - raise ArgumentError, 'Path contains non-UTF-8 byte sequence!' - end - end - - def directory? - blank_node? || @path.end_with?('/') - 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? && !blank_node?) ? name + ::File::SEPARATOR : name - end - - def name - @name || @path.split(::File::SEPARATOR).last.to_s - end - - def children - return [] unless directory? - return @children if @children - - child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$} - @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? - blank_node? || @universe.include?(@path) - end - - def blank_node? - @path.empty? # "" is considered to be './' - 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 -- cgit v1.2.1 From ad2b0358e0facd5c65c4141ce54c2e55bab165e6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 22:31:27 +0100 Subject: Improve readability of artifacts `Metadata` related code --- lib/gitlab/ci/build/artifacts/metadata.rb | 32 +++++++++---------------- lib/gitlab/ci/build/artifacts/metadata/entry.rb | 26 ++++++++++---------- 2 files changed, 24 insertions(+), 34 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index bfdfc9a1d7d..94821c0eae0 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -6,7 +6,9 @@ module Gitlab module Build module Artifacts class Metadata - VERSION_PATTERN = '[\w\s]+(\d+\.\d+\.\d+)' + VERSION_PATTERN = /^[\w\s]+(\d+\.\d+\.\d+)/ + INVALID_PATH_PATTERN = %r{(^\.?\.?/)|(/\.?\.?/)} + attr_reader :file, :path, :full_version def initialize(file, path) @@ -15,7 +17,7 @@ module Gitlab end def version - @full_version.match(/#{VERSION_PATTERN}/).captures.first + @full_version.match(VERSION_PATTERN)[1] end def errors @@ -27,7 +29,7 @@ module Gitlab end end - def match! + def find_entries! gzip do |gz| 2.times { read_string(gz) } # version and errors fields match_entries(gz) @@ -35,8 +37,8 @@ module Gitlab end def to_entry - entires, metadata = match! - Entry.new(@path, entires, metadata) + entries, metadata = find_entries! + Entry.new(@path, entries, metadata) end private @@ -44,7 +46,6 @@ module Gitlab def match_entries(gz) paths, metadata = [], [] match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$} - invalid_pattern = %r{(^\.?\.?/)|(/\.?\.?/)} until gz.eof? do begin @@ -53,7 +54,7 @@ module Gitlab next unless path.valid_encoding? && meta.valid_encoding? next unless path =~ match_pattern - next if path =~ invalid_pattern + next if path =~ INVALID_PATH_PATTERN paths.push(path) metadata.push(JSON.parse(meta, symbolize_names: true)) @@ -73,7 +74,7 @@ module Gitlab raise StandardError, 'Artifacts metadata file empty!' end - unless version_string =~ /^#{VERSION_PATTERN}/ + unless version_string =~ VERSION_PATTERN raise StandardError, 'Invalid version!' end @@ -92,19 +93,8 @@ module Gitlab gz.read(string_size) end - def gzip - open do |file| - gzip = Zlib::GzipReader.new(file) - begin - yield gzip - ensure - gzip.close - end - end - end - - def open - File.open(@file) { |file| yield file } + def gzip(&block) + Zlib::GzipReader.open(@file, &block) end end end diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb index 2fb6c327729..12bb1bf0346 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb @@ -11,12 +11,12 @@ module Gitlab # This class is working only with UTF-8 encoded paths. # class Entry - attr_reader :path, :entires + attr_reader :path, :entries attr_accessor :name - def initialize(path, entires, metadata = []) + def initialize(path, entries, metadata = []) @path = path.force_encoding('UTF-8') - @entires = entires + @entries = entries @metadata = metadata if path.include?("\0") @@ -42,7 +42,7 @@ module Gitlab def parent return nil unless has_parent? - new(@path.chomp(basename)) + new_entry(@path.chomp(basename)) end def basename @@ -58,7 +58,7 @@ module Gitlab return @children if @children child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$} - @children = select_entires { |entry| entry =~ child_pattern } + @children = select_entries { |entry| entry =~ child_pattern } end def directories(opts = {}) @@ -77,7 +77,7 @@ module Gitlab end def metadata - @index ||= @entires.index(@path) + @index ||= @entries.index(@path) @metadata[@index] || {} end @@ -90,7 +90,7 @@ module Gitlab end def exists? - blank_node? || @entires.include?(@path) + blank_node? || @entries.include?(@path) end def empty? @@ -102,7 +102,7 @@ module Gitlab end def ==(other) - @path == other.path && @entires == other.entires + @path == other.path && @entries == other.entries end def inspect @@ -111,13 +111,13 @@ module Gitlab private - def new(path) - self.class.new(path, @entires, @metadata) + def new_entry(path) + self.class.new(path, @entries, @metadata) end - def select_entires - selected = @entires.select { |entry| yield entry } - selected.map { |path| new(path) } + def select_entries + selected = @entries.select { |entry| yield entry } + selected.map { |path| new_entry(path) } end end end -- cgit v1.2.1 From 0d6e7b9d3d38e60e5a706956a853e7dc940e4574 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 23:24:28 +0100 Subject: Use Hash to store paths and entries metadata in artifacts browser --- lib/gitlab/ci/build/artifacts/metadata.rb | 11 +++++------ lib/gitlab/ci/build/artifacts/metadata/entry.rb | 18 ++++++------------ 2 files changed, 11 insertions(+), 18 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 94821c0eae0..25ecf27d4e6 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -37,14 +37,14 @@ module Gitlab end def to_entry - entries, metadata = find_entries! - Entry.new(@path, entries, metadata) + entries = find_entries! + Entry.new(@path, entries) end private def match_entries(gz) - paths, metadata = [], [] + entries = {} match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$} until gz.eof? do @@ -56,14 +56,13 @@ module Gitlab next unless path =~ match_pattern next if path =~ INVALID_PATH_PATTERN - paths.push(path) - metadata.push(JSON.parse(meta, symbolize_names: true)) + entries.store(path, JSON.parse(meta, symbolize_names: true)) rescue JSON::ParserError, Encoding::CompatibilityError next end end - [paths, metadata] + entries end def read_version diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb index 12bb1bf0346..4dae02ce4f7 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb @@ -14,10 +14,9 @@ module Gitlab attr_reader :path, :entries attr_accessor :name - def initialize(path, entries, metadata = []) - @path = path.force_encoding('UTF-8') + def initialize(path, entries) + @path = path.dup.force_encoding('UTF-8') @entries = entries - @metadata = metadata if path.include?("\0") raise ArgumentError, 'Path contains zero byte character!' @@ -42,7 +41,7 @@ module Gitlab def parent return nil unless has_parent? - new_entry(@path.chomp(basename)) + self.class.new(@path.chomp(basename), @entries) end def basename @@ -77,8 +76,7 @@ module Gitlab end def metadata - @index ||= @entries.index(@path) - @metadata[@index] || {} + @entries[@path] || {} end def nodes @@ -111,13 +109,9 @@ module Gitlab private - def new_entry(path) - self.class.new(path, @entries, @metadata) - end - def select_entries - selected = @entries.select { |entry| yield entry } - selected.map { |path| new_entry(path) } + selected = @entries.select { |entry, _metadata| yield entry } + selected.map { |path, _metadata| self.class.new(path, @entries) } end end end -- cgit v1.2.1 From be764a3a20c7cecce2a047ddd46aff954c33b306 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 14 Jan 2016 12:12:05 +0100 Subject: Minor improvements in build arfifacts browser Added also a `Gitlab::Ci::Build::Artifacts::Metadata::ParserError` exception class. --- lib/gitlab/ci/build/artifacts/metadata.rb | 17 ++++++++++++----- lib/gitlab/ci/build/artifacts/metadata/entry.rb | 4 ++-- 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index 25ecf27d4e6..1344f5d120b 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -6,6 +6,8 @@ module Gitlab module Build module Artifacts class Metadata + class ParserError < StandardError; end + VERSION_PATTERN = /^[\w\s]+(\d+\.\d+\.\d+)/ INVALID_PATH_PATTERN = %r{(^\.?\.?/)|(/\.?\.?/)} @@ -24,8 +26,13 @@ module Gitlab gzip do |gz| read_string(gz) # version errors = read_string(gz) - raise StandardError, 'Errors field not found!' unless errors - JSON.parse(errors) + raise ParserError, 'Errors field not found!' unless errors + + begin + JSON.parse(errors) + rescue JSON::ParserError + raise ParserError, 'Invalid errors field!' + end end end @@ -56,7 +63,7 @@ module Gitlab next unless path =~ match_pattern next if path =~ INVALID_PATH_PATTERN - entries.store(path, JSON.parse(meta, symbolize_names: true)) + entries[path] = JSON.parse(meta, symbolize_names: true) rescue JSON::ParserError, Encoding::CompatibilityError next end @@ -70,11 +77,11 @@ module Gitlab version_string = read_string(gz) unless version_string - raise StandardError, 'Artifacts metadata file empty!' + raise ParserError, 'Artifacts metadata file empty!' end unless version_string =~ VERSION_PATTERN - raise StandardError, 'Invalid version!' + raise ParserError, 'Invalid version!' end version_string.chomp diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb index 4dae02ce4f7..25b71fc3275 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb @@ -57,7 +57,7 @@ module Gitlab return @children if @children child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$} - @children = select_entries { |entry| entry =~ child_pattern } + @children = select_entries { |path| path =~ child_pattern } end def directories(opts = {}) @@ -110,7 +110,7 @@ module Gitlab private def select_entries - selected = @entries.select { |entry, _metadata| yield entry } + selected = @entries.select { |path, _metadata| yield path } selected.map { |path, _metadata| self.class.new(path, @entries) } end end -- cgit v1.2.1