[ARVADOS] updated: b92d4bfa6f4378756df5d28360bb708e9251917a
git at public.curoverse.com
git at public.curoverse.com
Thu Jun 11 20:16:06 EDT 2015
Summary of changes:
sdk/ruby/lib/arvados/keep.rb | 17 ++++---
sdk/ruby/test/test_keep_manifest.rb | 92 +++++++++++++++++++++++++++++--------
2 files changed, 84 insertions(+), 25 deletions(-)
via b92d4bfa6f4378756df5d28360bb708e9251917a (commit)
via deba5b86b2e0c41d18d6f9f7bd72f18755ed7a81 (commit)
from a0ce38b05f3392ad238d8a3b5049de9edbf4985f (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 b92d4bfa6f4378756df5d28360bb708e9251917a
Author: radhika <radhika at curoverse.com>
Date: Thu Jun 11 20:13:45 2015 -0400
6277: Add more restrictions to manifest format such as cannot start with '/' and end with '/' for file names,
as well as stream names and files names should not contain '//'.
Added the tests provided by Tom during review.
diff --git a/sdk/ruby/lib/arvados/keep.rb b/sdk/ruby/lib/arvados/keep.rb
index 0410589..bcab5fc 100644
--- a/sdk/ruby/lib/arvados/keep.rb
+++ b/sdk/ruby/lib/arvados/keep.rb
@@ -97,8 +97,8 @@ module Keep
end
class Manifest
- STREAM_REGEXP = /(\.)(\/+.*)*$/
- FILE_REGEXP = /^[[:digit:]]+:[[:digit:]]+:/
+ STREAM_REGEXP = /(\.)((\/+.*[^\/])*)$/
+ FILE_REGEXP = /^[[:digit:]]+:[[:digit:]]+:(?!\/).*[^\/]$/
# Class to parse a manifest text and provide common views of that data.
def initialize(manifest_text)
@@ -237,12 +237,14 @@ module Keep
manifest.each_line do |line|
line_count += 1
- words = line.split
+ words = line.split(/[[:space:]]/)
+ raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing stream name" if words.empty?
count = 0
+
word = words.shift
- count += 1 if word =~ STREAM_REGEXP
- raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid stream name #{word.inspect}" if count != 1
+ count += 1 if word =~ STREAM_REGEXP and !word.include? '//'
+ raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid stream name #{word.inspect if word}" if count != 1
count = 0
word = words.shift
@@ -250,10 +252,10 @@ module Keep
word = words.shift
count += 1
end
- raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid locator #{word.inspect}" if count == 0
+ raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid locator #{word.inspect if word}" if count == 0
count = 0
- while word =~ FILE_REGEXP
+ while(word =~ FILE_REGEXP and !word.include? '//')
word = words.shift
count += 1
end
diff --git a/sdk/ruby/test/test_keep_manifest.rb b/sdk/ruby/test/test_keep_manifest.rb
index 3de34f0..18a2b06 100644
--- a/sdk/ruby/test/test_keep_manifest.rb
+++ b/sdk/ruby/test/test_keep_manifest.rb
@@ -281,24 +281,79 @@ class ManifestTest < Minitest::Test
end
[
- [false, '. d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt', "Invalid manifest: does not end with new line"],
- [false, "abc d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n", "invalid stream name \"abc\""],
- [false, "d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n", "invalid stream name \"d41d8cd98f00b204e9800998ecf8427e+0\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427 0:0:abc.txt\n", "invalid locator \"d41d8cd98f00b204e9800998ecf8427\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e\n", "Manifest invalid for stream 1: no file tokens"],
- [true, ". d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n"],
- [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"],
- [true, ". d41d8cd98f00b204e9800998ecf8427e a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"], # 2 locators
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n/dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
- "Manifest invalid for stream 2: missing or invalid stream name \"/dir1\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
- "Manifest invalid for stream 2: missing or invalid locator \"d41d8cd98f00b204e9800998ecf842\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 abc.txt\n",
- "Manifest invalid for stream 2: invalid file token \"abc.txt\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:abc.txt\n",
- "Manifest invalid for stream 2: invalid file token \"0:abc.txt\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt xyz.txt\n",
- "Manifest invalid for stream 2: invalid file token \"xyz.txt\""],
+ [false, '. d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt',
+ "Invalid manifest: does not end with new line"],
+ [false, "abc d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n",
+ "invalid stream name \"abc\""],
+ [false, "d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n",
+ "invalid stream name \"d41d8cd98f00b204e9800998ecf8427e+0\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427 0:0:abc.txt\n",
+ "invalid locator \"d41d8cd98f00b204e9800998ecf8427\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e\n",
+ "Manifest invalid for stream 1: no file tokens"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"], # 2 locators
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n/dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
+ "Manifest invalid for stream 2: missing or invalid stream name \"/dir1\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
+ "Manifest invalid for stream 2: missing or invalid locator \"d41d8cd98f00b204e9800998ecf842\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 abc.txt\n",
+ "Manifest invalid for stream 2: invalid file token \"abc.txt\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:abc.txt\n",
+ "Manifest invalid for stream 2: invalid file token \"0:abc.txt\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt xyz.txt\n",
+ "Manifest invalid for stream 2: invalid file token \"xyz.txt\""],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo/bar.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 000000000000000000000000000000:0777:foo.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:0:0\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:\\040\n"],
+ [true, ". 00000000000000000000000000000000+0 0:0:0\n"],
+ [true, ". 00000000000000000000000000000000+0 0:0:d41d8cd98f00b204e9800998ecf8427e+0+Ad41d8cd98f00b204e9800998ecf8427e00000000 at ffffffff\n"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt d41d8cd98f00b204e9800998ecf8427e+0\n",
+ "Manifest invalid for stream 1: invalid file token \"d41d8cd98f00b204e9800998ecf8427e+0\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:\n",
+ "Manifest invalid for stream 1: invalid file token \"0:0:\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0\n",
+ "Manifest invalid for stream 1: no file tokens"],
+ [false, ". 0:0:foo.txt d41d8cd98f00b204e9800998ecf8427e+0\n",
+ "Manifest invalid for stream 1: missing or invalid locator \"0:0:foo.txt\""],
+ [false, ". 0:0:foo.txt\n", "Manifest invalid for stream 1: missing or invalid locator \"0:0:foo.txt\""],
+ [false, ".\n", "Manifest invalid for stream 1: missing or invalid locator"],
+ [false, ".", "Invalid manifest: does not end with new line"],
+ [false, ". \n", "Manifest invalid for stream 1: missing or invalid locator"],
+ [false, ". \n", "Manifest invalid for stream 1: missing or invalid locator"],
+# This does not fail since \t is interpreted as one white space character and hence is split upon
+# [false, ".\td41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n", "Manifest invalid for stream 1: no file tokens"],
+ [false, " . d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n", "Manifest invalid for stream 1: missing or invalid stream name"],
+# This does not fail since the white space after foo.txt is used to split upon
+# [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt \n", "Manifest invalid for stream 1: no file tokens"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: invalid file token"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n \n",
+ "Manifest invalid for stream 2: missing stream name"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n\n",
+ "Manifest invalid for stream 2: missing stream name"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n ",
+ "Invalid manifest: does not end with new line"],
+ [false, "\n. d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing stream name"],
+ [false, " \n. d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing stream name"],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:/foo.txt\n",
+ "Manifest invalid for stream 1: invalid file token \"0:0:/foo.txt\""],
+ [false, "./ d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing or invalid stream name \"./\""],
+ [false, ".//foo d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing or invalid stream name \".//foo\""],
+ [false, "./foo/ d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing or invalid stream name \"./foo/\""],
+ [false, "./foo//bar d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo.txt\n",
+ "Manifest invalid for stream 1: missing or invalid stream name \"./foo//bar\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo//bar.txt\n",
+ "Manifest invalid for stream 1: invalid file token \"0:0:foo//bar.txt\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo/\n",
+ "Manifest invalid for stream 1: invalid file token \"0:0:foo/\""],
].each do |ok, manifest, expected_error=nil|
define_method "test_manifest_valid_#{ok}_#{manifest}_and_expect_error_#{expected_error}" do
if ok
@@ -310,7 +365,7 @@ class ManifestTest < Minitest::Test
msg = e.message
end
refute_nil msg, "Expected ArgumentError"
- assert msg.include?(expected_error), "Did not find error message #{expected_error} in #{msg}"
+ assert msg.include?(expected_error), "Did not find expected error message. Expected: #{expected_error}; Actual: #{msg}"
end
end
end
commit deba5b86b2e0c41d18d6f9f7bd72f18755ed7a81
Author: radhika <radhika at curoverse.com>
Date: Thu Jun 11 16:19:05 2015 -0400
6277: valid manifest must end with new line.
diff --git a/sdk/ruby/lib/arvados/keep.rb b/sdk/ruby/lib/arvados/keep.rb
index 2006d33..0410589 100644
--- a/sdk/ruby/lib/arvados/keep.rb
+++ b/sdk/ruby/lib/arvados/keep.rb
@@ -232,6 +232,7 @@ module Keep
# Valid format: stream name + one or more locators + one or more files for each stream in manifest.
# https://arvados.org/projects/arvados/wiki/Keep_manifest_format
def self.valid?(manifest)
+ raise ArgumentError.new "Invalid manifest: does not end with new line" if !manifest.end_with?("\n")
line_count = 0
manifest.each_line do |line|
line_count += 1
diff --git a/sdk/ruby/test/test_keep_manifest.rb b/sdk/ruby/test/test_keep_manifest.rb
index 08965fa..3de34f0 100644
--- a/sdk/ruby/test/test_keep_manifest.rb
+++ b/sdk/ruby/test/test_keep_manifest.rb
@@ -281,22 +281,23 @@ class ManifestTest < Minitest::Test
end
[
- [false, 'abc d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt', "invalid stream name \"abc\""],
- [false, 'd41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt', "invalid stream name \"d41d8cd98f00b204e9800998ecf8427e+0\""],
- [false, '. d41d8cd98f00b204e9800998ecf8427 0:0:abc.txt', "invalid locator \"d41d8cd98f00b204e9800998ecf8427\""],
- [false, '. d41d8cd98f00b204e9800998ecf8427e', "Manifest invalid for stream 1: no file tokens"],
- [true, '. d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt'],
- [true, '. d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt'],
- [true, '. d41d8cd98f00b204e9800998ecf8427e a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt'], # 2 locators
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n/dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt",
+ [false, '. d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt', "Invalid manifest: does not end with new line"],
+ [false, "abc d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n", "invalid stream name \"abc\""],
+ [false, "d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n", "invalid stream name \"d41d8cd98f00b204e9800998ecf8427e+0\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427 0:0:abc.txt\n", "invalid locator \"d41d8cd98f00b204e9800998ecf8427\""],
+ [false, ". d41d8cd98f00b204e9800998ecf8427e\n", "Manifest invalid for stream 1: no file tokens"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"],
+ [true, ". d41d8cd98f00b204e9800998ecf8427e a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n"], # 2 locators
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n/dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
"Manifest invalid for stream 2: missing or invalid stream name \"/dir1\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt",
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 d41d8cd98f00b204e9800998ecf842 0:0:abc.txt\n",
"Manifest invalid for stream 2: missing or invalid locator \"d41d8cd98f00b204e9800998ecf842\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 abc.txt",
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 abc.txt\n",
"Manifest invalid for stream 2: invalid file token \"abc.txt\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:abc.txt",
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:abc.txt\n",
"Manifest invalid for stream 2: invalid file token \"0:abc.txt\""],
- [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt xyz.txt",
+ [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt\n./dir1 a41d8cd98f00b204e9800998ecf8427e+0 0:0:abc.txt xyz.txt\n",
"Manifest invalid for stream 2: invalid file token \"xyz.txt\""],
].each do |ok, manifest, expected_error=nil|
define_method "test_manifest_valid_#{ok}_#{manifest}_and_expect_error_#{expected_error}" do
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list