From 32eafceeb044696ac7db49bbe1d6329e0e3785c0 Mon Sep 17 00:00:00 2001 From: Tim Pierce Date: Wed, 21 May 2014 13:34:22 -0400 Subject: [PATCH] 2755: incorporate code review * lib/locator.rb renamed => app/models/locator.rb * Relaxed Locator.parse! handling of hint content. * Locator.parse() rescues only from ArgumentError. * Removed blob_signing_ttl from application.yml.example. * Collections.show only matches locators that are preceded by a space, when parsing manifest_text. --- .../arvados/v1/collections_controller.rb | 14 ++-- services/api/app/models/locator.rb | 84 +++++++++++++++++++ services/api/config/application.yml.example | 3 - services/api/lib/locator.rb | 63 -------------- 4 files changed, 90 insertions(+), 74 deletions(-) create mode 100644 services/api/app/models/locator.rb delete mode 100644 services/api/lib/locator.rb diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb index 2844cb4c7a..1982a525f7 100644 --- a/services/api/app/controllers/arvados/v1/collections_controller.rb +++ b/services/api/app/controllers/arvados/v1/collections_controller.rb @@ -1,5 +1,3 @@ -require 'locator' - class Arvados::V1::CollectionsController < ApplicationController def create # Collections are owned by system_user. Creating a collection has @@ -24,8 +22,8 @@ class Arvados::V1::CollectionsController < ApplicationController } resource_attrs[:manifest_text].lines.each do |entry| entry.split[1..-1].each do |tok| - # TODO(twp): fail the request if this match fails. - # Add in Phase 4 (see #2755) + # TODO(twp): in Phase 4, fail the request if the locator + # lacks a permission signature. (see #2755) loc = Locator.parse(tok) if loc and loc.signature if !api_token @@ -44,10 +42,10 @@ class Arvados::V1::CollectionsController < ApplicationController # Remove any permission signatures from the manifest. resource_attrs[:manifest_text] - .gsub!(/[[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word| + .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word| loc = Locator.parse(word) if loc - loc.without_signature.to_s + " " + loc.without_signature.to_s else word end @@ -95,10 +93,10 @@ class Arvados::V1::CollectionsController < ApplicationController ttl: Rails.configuration.blob_signing_ttl, } @object[:manifest_text] - .gsub!(/[[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word| + .gsub!(/ [[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)/) { |word| loc = Locator.parse(word) if loc - Blob.sign_locator(word, signing_opts) + " " + Blob.sign_locator(word, signing_opts) else word end diff --git a/services/api/app/models/locator.rb b/services/api/app/models/locator.rb new file mode 100644 index 0000000000..39d7da94dc --- /dev/null +++ b/services/api/app/models/locator.rb @@ -0,0 +1,84 @@ +# A Locator is used to parse and manipulate Keep locator strings. +# +# Locators obey the following syntax: +# +# locator ::= address hint* +# address ::= digest size-hint +# digest ::= <32 hexadecimal digits> +# size-hint ::= "+" [0-9]+ +# hint ::= "+" hint-type hint-content +# hint-type ::= [A-Z] +# hint-content ::= [A-Za-z0-9@_-]+ +# +# Individual hints may have their own required format: +# +# sign-hint ::= "+A" <40 lowercase hex digits> "@" sign-timestamp +# sign-timestamp ::= <8 lowercase hex digits> + +class Locator + def initialize(hasharg, sizearg, hintarg) + @hash = hasharg + @size = sizearg + @hints = hintarg + end + + # Locator.parse returns a Locator object parsed from the string tok. + # Returns nil if tok could not be parsed as a valid locator. + def self.parse(tok) + begin + Locator.parse!(tok) + rescue ArgumentError => e + nil + end + end + + # Locator.parse! returns a Locator object parsed from the string tok, + # raising an ArgumentError if tok cannot be parsed. + def self.parse!(tok) + m = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?(\+([[:upper:]][[:alnum:]+@_-]*))?$/.match(tok.strip) + unless m + raise ArgumentError.new "could not parse #{tok}" + end + + tokhash, _, toksize, _, trailer = m[1..5] + tokhints = [] + if trailer + trailer.split('+').each do |hint| + if hint =~ /^[[:upper:]][[:alnum:]@_-]+$/ + tokhints.push(hint) + else + raise ArgumentError.new "unknown hint #{hint}" + end + end + end + + Locator.new(tokhash, toksize, tokhints) + end + + # Returns the signature hint supplied with this locator, + # or nil if the locator was not signed. + def signature + @hints.grep(/^A/).first + end + + # Returns an unsigned Locator. + def without_signature + Locator.new(@hash, @size, @hints.reject { |o| o.start_with?("A") }) + end + + def hash + @hash + end + + def size + @size + end + + def hints + @hints + end + + def to_s + [ @hash, @size, *@hints ].join('+') + end +end diff --git a/services/api/config/application.yml.example b/services/api/config/application.yml.example index e0420ce076..030e23894f 100644 --- a/services/api/config/application.yml.example +++ b/services/api/config/application.yml.example @@ -52,6 +52,3 @@ common: #git_repositories_dir: /var/cache/git #git_internal_dir: /var/cache/arvados/internal.git - # Amount of time (in seconds) for which a blob permission signature - # remains valid. Default: 2 weeks (1209600 seconds) - #blob_signing_ttl: 1209600 diff --git a/services/api/lib/locator.rb b/services/api/lib/locator.rb deleted file mode 100644 index 0ec3f625ef..0000000000 --- a/services/api/lib/locator.rb +++ /dev/null @@ -1,63 +0,0 @@ -class Locator - # This regex will match a word that appears to be a locator. - @@pattern = %r![[:xdigit:]]{32}(\+[[:digit:]]+)?(\+\S+)?! - - def initialize(hasharg, sizearg, optarg) - @hash = hasharg - @size = sizearg - @options = optarg - end - - def self.parse(tok) - Locator.parse!(tok) rescue nil - end - - def self.parse!(tok) - m = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?(\+.*)?$/.match(tok) - unless m - raise ArgumentError.new "could not parse #{tok}" - end - - tokhash, _, toksize, trailer = m[1..4] - tokopts = [] - while m = /^\+[[:upper:]][^\s+]+/.match(trailer) - opt = m.to_s - if opt =~ /^\+A[[:xdigit:]]+@[[:xdigit:]]{8}$/ or - opt =~ /\+K@[[:alnum:]]+$/ - tokopts.push(opt) - trailer = m.post_match - else - raise ArgumentError.new "unknown option #{opt}" - end - end - if trailer and !trailer.empty? - raise ArgumentError.new "unrecognized trailing chars #{trailer}" - end - - Locator.new(tokhash, toksize, tokopts) - end - - def signature - @options.grep(/^\+A/).first - end - - def without_signature - Locator.new(@hash, @size, @options.reject { |o| o.start_with?("+A") }) - end - - def hash - @hash - end - - def size - @size - end - - def options - @options - end - - def to_s - [ @hash + "+", @size, *@options].join - end -end -- 2.30.2