14482: Handle escaped chars on manifest validation.

* Escaped & unescaped chars are treated equally, except for \040.
* Empty '.' (and \056) named file tokens are accepted.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima@veritasgenetics.com>

diff --git a/sdk/ruby/lib/arvados/keep.rb b/sdk/ruby/lib/arvados/keep.rb
index ce1331ac1c8edf94444356effe13fad18abcd677..2d444d074d82f29cc216fb74b1ef2a3b656d5269 100644 (file)
--- a/sdk/ruby/lib/arvados/keep.rb
+++ b/sdk/ruby/lib/arvados/keep.rb
@@ -103,6 +103,7 @@ module Keep
   class Manifest
     STRICT_STREAM_TOKEN_REGEXP = /^(\.)(\/[^\/\s]+)*$/
     STRICT_FILE_TOKEN_REGEXP = /^[[:digit:]]+:[[:digit:]]+:([^\s\/]+(\/[^\s\/]+)*)$/
+    EMPTY_DOT_FILE_TOKEN_REGEXP = /^0:0:\.$/
 
     # Class to parse a manifest text and provide common views of that data.
     def initialize(manifest_text)
@@ -131,18 +132,25 @@ module Keep
       end
     end
 
-    def unescape(s)
+    def self.unescape(s, except=[])
+      return nil if s.nil?
+
       # Parse backslash escapes in a Keep manifest stream or file name.
       s.gsub(/\\(\\|[0-7]{3})/) do |_|
-        case $1
-        when '\\'
+        if $1 == '\\'
           '\\'
+        elsif except.include? $1
+          $1
         else
           $1.to_i(8).chr
         end
       end
     end
 
+    def unescape(s)
+      self.class.unescape(s)
+    end
+
     def split_file_token token
       start_pos, filesize, filename = token.split(':', 3)
       if filename.nil?
@@ -250,7 +258,8 @@ module Keep
         count = 0
 
         word = words.shift
-        count += 1 if word =~ STRICT_STREAM_TOKEN_REGEXP and word !~ /\/\.\.?(\/|$)/
+        unescaped_word = unescape(word, except=["040"])
+        count += 1 if unescaped_word =~ STRICT_STREAM_TOKEN_REGEXP and unescaped_word !~ /\/\.\.?(\/|$)/
         raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid stream name #{word.inspect if word}" if count != 1
 
         count = 0
@@ -262,7 +271,8 @@ module Keep
         raise ArgumentError.new "Manifest invalid for stream #{line_count}: missing or invalid locator #{word.inspect if word}" if count == 0
 
         count = 0
-        while word =~ STRICT_FILE_TOKEN_REGEXP and ($~[1].split('/') & ['..','.']).empty?
+        while unescape(word) =~ EMPTY_DOT_FILE_TOKEN_REGEXP or
+          (unescape(word, except=["040"]) =~ STRICT_FILE_TOKEN_REGEXP and ($~[1].split('/') & ['..', '.']).empty?)
           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 71a24a28c08dea61a186161d6168e908f7cd4404..bd13127018c05a8ae171e04159d0d16f4f5db539 100644 (file)
--- a/sdk/ruby/test/test_keep_manifest.rb
+++ b/sdk/ruby/test/test_keep_manifest.rb
@@ -320,6 +320,7 @@ class ManifestTest < Minitest::Test
     [true, ". 00000000000000000000000000000000+0 0:0:0\n"],
     [true, ". 00000000000000000000000000000000+0 0:0:d41d8cd98f00b204e9800998ecf8427e+0+Ad41d8cd98f00b204e9800998ecf8427e00000000@ffffffff\n"],
     [true, ". d41d8cd98f00b204e9800998ecf8427e+0+Ad41d8cd98f00b204e9800998ecf8427e00000000@ffffffff 0:0:empty.txt\n"],
+    [true, "./empty_dir d41d8cd98f00b204e9800998ecf8427e+0 0:0:.\n"],
     [false, '. d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt',
       "Invalid manifest: does not end with newline"],
     [false, "abc d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n",
@@ -334,8 +335,9 @@ class ManifestTest < Minitest::Test
       "invalid stream name \"./abc/..\""],
     [false, "./abc/./foo d41d8cd98f00b204e9800998ecf8427e 0:0:abc.txt\n",
       "invalid stream name \"./abc/./foo\""],
-    [false, ". d41d8cd98f00b204e9800998ecf8427e 0:0:.\n",
-      "invalid file token \"0:0:.\""],
+    # non-empty '.'-named file tokens aren't acceptable. Empty ones are used as empty dir placeholders.
+    [false, ". 8cf8463b34caa8ac871a52d5dd7ad1ef+1 0:1:.\n",
+      "invalid file token \"0:1:.\""],
     [false, ". d41d8cd98f00b204e9800998ecf8427e 0:0:..\n",
       "invalid file token \"0:0:..\""],
     [false, ". d41d8cd98f00b204e9800998ecf8427e 0:0:./abc.txt\n",
@@ -429,6 +431,18 @@ class ManifestTest < Minitest::Test
       "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/\""],
+    # escaped chars
+    [true, "./empty_dir d41d8cd98f00b204e9800998ecf8427e+0 0:0:\\056\n"],
+    [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\\057bar\n"],
+    [true, ".\\057Data d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n"],
+    [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\\057/bar\n",
+      "Manifest invalid for stream 1: invalid file token \"0:0:foo\\\\057/bar\""],
+    [false, ".\\057/Data d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n",
+      "Manifest invalid for stream 1: missing or invalid stream name \".\\\\057/Data\""],
+    [true, "./Data\\040Folder d41d8cd98f00b204e9800998ecf8427e+0 0:0:foo\n"],
+    [false, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:\\057foo/bar\n",
+      "Manifest invalid for stream 1: invalid file token \"0:0:\\\\057foo/bar\""],
+    [true, ". d41d8cd98f00b204e9800998ecf8427e+0 0:0:\\134057foo/bar\n"],
   ].each do |ok, manifest, expected_error=nil|
     define_method "test_validate manifest #{manifest.inspect}" do
       assert_equal ok, Keep::Manifest.valid?(manifest)