[ARVADOS] updated: bed1ec60de09b319fccd37e7452280736a7c5a8a

[git at public.curoverse.com]

Summary of changes:
 sdk/ruby/lib/arvados/keep.rb                       |  2 +-
 sdk/ruby/test/test_keep_manifest.rb                | 34 ++++++++++++++++++++++
 services/api/app/models/collection.rb              | 10 +++++--
 .../arvados/v1/collections_controller_test.rb      | 14 +++++++--
 4 files changed, 54 insertions(+), 6 deletions(-)

       via  bed1ec60de09b319fccd37e7452280736a7c5a8a (commit)
       via  dad32377053298a53ec1f8ac3ce184211f6bdc49 (commit)
       via  43cfd4220fea2ba6632bbb82e4d1779c68d827fc (commit)
      from  0477856a1726b2e05c3ae69318e4ff5fd210d77c (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit bed1ec60de09b319fccd37e7452280736a7c5a8a
Merge: 0477856 dad3237
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Jun 9 10:11:35 2015 -0400

    Merge branch '6203-locator-regexp' refs #6203 refs #6277


commit dad32377053298a53ec1f8ac3ce184211f6bdc49
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Jun 8 15:34:48 2015 -0400

    6203: Fix loophole allowing locators in bogus manifests to be accepted
    without verification and then signed in responses. refs #6277

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index bbced41..1fa7df6 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -62,8 +62,10 @@ class Collection < ArvadosModel
         now: db_current_time.to_i,
       }
       self.manifest_text.each_line do |entry|
-        entry.split[1..-1].each do |tok|
-          if tok =~ FILE_TOKEN
+        entry.split.each do |tok|
+          if tok == '.' or tok.starts_with? './'
+            # Stream name token.
+          elsif tok =~ FILE_TOKEN
             # This is a filename token, not a blob locator. Note that we
             # keep checking tokens after this, even though manifest
             # format dictates that all subsequent tokens will also be
@@ -220,7 +222,9 @@ class Collection < ArvadosModel
       line.rstrip!
       new_words = []
       line.split(' ').each do |word|
-        if match = Keep::Locator::LOCATOR_REGEXP.match(word)
+        if new_words.empty?
+          new_words << word
+        elsif match = Keep::Locator::LOCATOR_REGEXP.match(word)
           new_words << yield(match)
         else
           new_words << word
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index d2901a0..1418f69 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -531,8 +531,8 @@ EOS
     }
 
     # Generate a locator with a bad signature.
-    unsigned_locator = "d41d8cd98f00b204e9800998ecf8427e+0"
-    bad_locator = unsigned_locator + "+Affffffff at ffffffff"
+    unsigned_locator = "acbd18db4cc2f85cedef654fccc4a4d8+3"
+    bad_locator = unsigned_locator + "+Affffffffffffffffffffffffffffffffffffffff at ffffffff"
     assert !Blob.verify_signature(bad_locator, signing_opts)
 
     # Creating a collection with this locator should
@@ -577,6 +577,16 @@ EOS
     assert_response 422
   end
 
+  test "reject manifest with unsigned block as stream name" do
+    authorize_with :active
+    post :create, {
+      collection: {
+        manifest_text: "00000000000000000000000000000000+1234 d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n"
+      }
+    }
+    assert_includes [422, 403], response.code.to_i
+  end
+
   test "multiple locators per line" do
     permit_unsigned_manifests
     authorize_with :active

commit 43cfd4220fea2ba6632bbb82e4d1779c68d827fc
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Jun 8 12:03:42 2015 -0400

    6203: Add tests for LOCATOR_REGEXP. Fix regexp to reject "++" and trailing "\n".

diff --git a/sdk/ruby/lib/arvados/keep.rb b/sdk/ruby/lib/arvados/keep.rb
index 422dab5..739ff01 100644
--- a/sdk/ruby/lib/arvados/keep.rb
+++ b/sdk/ruby/lib/arvados/keep.rb
@@ -18,7 +18,7 @@ module Keep
     #   sign-timestamp ::= <8 lowercase hex digits>
     attr_reader :hash, :hints, :size
 
-    LOCATOR_REGEXP = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?(\+([[:upper:]][[:alnum:]+ at _-]*))?$/
+    LOCATOR_REGEXP = /^([[:xdigit:]]{32})(\+([[:digit:]]+))?((\+([[:upper:]][[:alnum:]@_-]*))+)?\z/
 
     def initialize(hasharg, sizearg, hintarg)
       @hash = hasharg
diff --git a/sdk/ruby/test/test_keep_manifest.rb b/sdk/ruby/test/test_keep_manifest.rb
index 70bae3e..7689d50 100644
--- a/sdk/ruby/test/test_keep_manifest.rb
+++ b/sdk/ruby/test/test_keep_manifest.rb
@@ -245,4 +245,38 @@ class ManifestTest < Minitest::Test
       assert_equal(%w(file1), basenames.sort, "wrong file list for #{stream}")
     end
   end
+
+  [[false, nil],
+   [false, '+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e0+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0 '],
+   [false, "d41d8cd98f00b204e9800998ecf8427e+0\n"],
+   [false, ' d41d8cd98f00b204e9800998ecf8427e+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+K+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0+0'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e++'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0+K+'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0++K'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0+K++'],
+   [false, 'd41d8cd98f00b204e9800998ecf8427e+0+K++Z'],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e', nil,nil,nil],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e+0', '+0','0',nil],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e+0+Fizz+Buzz','+0','0','+Fizz+Buzz'],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e+Fizz+Buzz', nil,nil,'+Fizz+Buzz'],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e+0+Z', '+0','0','+Z'],
+   [true, 'd41d8cd98f00b204e9800998ecf8427e+Z', nil,nil,'+Z'],
+  ].each do |ok, locator, match2, match3, match4|
+    define_method "test_LOCATOR_REGEXP_on_#{locator.inspect}" do
+      match = Keep::Locator::LOCATOR_REGEXP.match locator
+      assert_equal ok, !!match
+      if ok
+        assert_equal match2, match[2]
+        assert_equal match3, match[3]
+        assert_equal match4, match[4]
+      end
+    end
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
--