Strengthen checks when creating paths, fix some bugs, and add tests

nilmdb/server/bulkdata.py

@@ -56,6 +56,50 @@ class BulkData(object):
             return path.encode('utf-8')
         return path
 
+    def _create_parents(self, unicodepath):
+        """Verify the path name, and create parent directories if they
+        don't exist.  Returns a list of elements that got created."""
+        path = self._encode_filename(unicodepath)
+
+        if path[0] != '/':
+            raise ValueError("paths must start with /")
+        [ group, node ] = path.rsplit("/", 1)
+        if group == '':
+            raise ValueError("invalid path; path must contain at least one "
+                             "folder")
+        if node == '':
+            raise ValueError("invalid path; should not end with a /")
+        if not Table.valid_path(path):
+            raise ValueError("path name is invalid or contains reserved words")
+
+        # Create the table.  Note that we make a distinction here
+        # between NilmDB paths (always Unix style, split apart
+        # manually) and OS paths (built up with os.path.join)
+
+        # Make directories leading up to this one
+        elements = path.lstrip('/').split('/')
+        made_dirs = []
+        try:
+            # Make parent elements
+            for i in range(len(elements)):
+                ospath = os.path.join(self.root, *elements[0:i])
+                if Table.exists(ospath):
+                    raise ValueError("path is subdir of existing node")
+                if not os.path.isdir(ospath):
+                    os.mkdir(ospath)
+                    made_dirs.append(ospath)
+        except Exception as e:
+            # Try to remove paths that we created; ignore errors
+            exc_info = sys.exc_info()
+            for ospath in reversed(made_dirs):
+                try:
+                    os.rmdir(ospath)
+                except OSError:
+                    pass
+            raise exc_info[1], None, exc_info[2]
+
+        return elements
+
     def create(self, unicodepath, layout_name):
         """
         unicodepath: path to the data (e.g. u'/newton/prep').
@@ -67,27 +111,7 @@ class BulkData(object):
 
         layout_name: string for nilmdb.layout.get_named(), e.g. 'float32_8'
         """
-        path = self._encode_filename(unicodepath)
-
-        if path[0] != '/':
-            raise ValueError("paths must start with /")
-        [ group, node ] = path.rsplit("/", 1)
-        if group == '':
-            raise ValueError("invalid path; path must contain at least one "
-                             "folder")
-
-        # Create the table.  Note that we make a distinction here
-        # between NilmDB paths (always Unix style, split apart
-        # manually) and OS paths (built up with os.path.join)
-
-        # Make directories leading up to this one
-        elements = path.lstrip('/').split('/')
-        for i in range(len(elements)):
-            ospath = os.path.join(self.root, *elements[0:i])
-            if Table.exists(ospath):
-                raise ValueError("path is subdir of existing node")
-            if not os.path.isdir(ospath):
-                os.mkdir(ospath)
+        elements = self._create_parents(unicodepath)
 
         # Make the final dir
         ospath = os.path.join(self.root, *elements)
@@ -161,6 +185,11 @@ class Table(object):
     # See design.md for design details
 
     # Class methods, to help keep format details in this class.
+    @classmethod
+    def valid_path(cls, root):
+        """Return True if a root path is a valid name"""
+        return "_format" not in root.split("/")
+
     @classmethod
     def exists(cls, root):
         """Return True if a table appears to exist at this OS path"""

tests/test_cmdline.py

@@ -250,6 +250,15 @@ class TestCmdline(object):
 
         self.fail("create /foo float32_8")
         self.contain("invalid path")
+        self.fail("create /newton/prep/ float32_8")
+        self.contain("invalid path")
+
+        self.fail("create /newton/_format/prep float32_8")
+        self.contain("path name is invalid")
+        self.fail("create /_format/newton/prep float32_8")
+        self.contain("path name is invalid")
+        self.fail("create /newton/prep/_format float32_8")
+        self.contain("path name is invalid")
 
         # Bad layout type
         self.fail("create /newton/prep NoSuchLayout")