6277: more tests with error checking

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 99a09bd8e5d885324c1b073975a1c92ebc88ff5b..f86cf22403ec44fcd21523ede1de5af91407d90e 100644 (file)
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -172,6 +172,7 @@ class Collection < ArvadosModel
   end
 
   def check_encoding
+    return true if !manifest_text
     if manifest_text.encoding.name == 'UTF-8' and manifest_text.valid_encoding?
       true
     else
@@ -198,7 +199,7 @@ class Collection < ArvadosModel
       Keep::Manifest.validate! manifest_text
       true
     rescue => e
-      logger.warn e
+      errors.add :manifest_text, e.message
       false
     end
   end

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 c521e590cc8c788709bf594afd79a016dbe5658a..dac960f5d81e5db90331c1978f0e3adffc0fc5b5 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -826,6 +826,43 @@ EOS
         }
       }
       assert_response 422
+      response_errors = json_response['errors']
+      assert_not_nil response_errors, 'Expected error in response'
+      if manifest_text
+        assert(response_errors.first.include?('Invalid manifest'),
+               "Expected 'Invalid manifest' error in #{response_errors.first}")
+      else
+        assert(response_errors.first.include?('No manifest found'),
+               "Expected 'No manifest found' error in #{response_errors.first}")
+      end
+    end
+  end
+
+  [
+    nil,
+    ". 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e foo.txt",
+    "d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+  ].each do |manifest_text|
+    test "update collection with invalid manifest #{manifest_text}" do
+      authorize_with :active
+      post :update, {
+        id: 'zzzzz-4zz18-bv31uwvy3neko21',
+        collection: {
+          manifest_text: manifest_text,
+        }
+      }
+      assert_response 422
+      response_errors = json_response['errors']
+      assert_not_nil response_errors, 'Expected error in response'
+      if manifest_text
+        assert(response_errors.first.include?('Invalid manifest'),
+               "Expected 'Invalid manifest' error in #{response_errors.first}")
+      else
+        assert(response_errors.first.include?('No manifest found'),
+               "Expected 'No manifest found' error in #{response_errors.first}")
+      end
     end
   end
 end

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index bcdefe75afb222181ed4c6afb9b45d5bc9bca4db..d6b50da140a86004c78ad9ff36158f77767c939f 100644 (file)
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -39,6 +39,39 @@ class CollectionTest < ActiveSupport::TestCase
     end
   end
 
+  [
+    nil,
+    ". 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e foo.txt",
+    "d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+  ].each do |manifest_text|
+    test "create collection with invalid manifest text #{manifest_text} and expect error" do
+      act_as_system_user do
+        c = Collection.create(manifest_text: manifest_text)
+        assert !c.valid?
+      end
+    end
+  end
+
+  [
+    nil,
+    ". 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e foo.txt",
+    "d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+    ". d41d8cd98f00b204e9800998ecf8427e 0:0:foo.txt",
+  ].each do |manifest_text|
+    test "update collection with invalid manifest text #{manifest_text} and expect error" do
+      act_as_system_user do
+        c = create_collection 'foo', Encoding::US_ASCII
+        assert c.valid?
+
+        c.update_attribute 'manifest_text', manifest_text
+        assert !c.valid?
+      end
+    end
+  end
+
   test 'create and update collection and verify file_names' do
     act_as_system_user do
       c = create_collection 'foo', Encoding::US_ASCII

diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index de12fcce1763764e5ef80e6ef2ce62d70178b9d6..8ffca49601d795fc0622326e04eb219dd3dd0d8d 100644 (file)
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -23,6 +23,7 @@ _logger = logging.getLogger('arvados.arvados_fuse')
 # appear as underscores in the fuse mount.)
 _disallowed_filename_characters = re.compile('[\x00/]')
 
+# '.' and '..' are not reachable if API server is newer than #6277
 def sanitize_filename(dirty):
     """Replace disallowed filename characters with harmless "_"."""
     if dirty is None: