Docserver: Fix invalid path usage in CloudStorageFileSystem.

chrome/common/extensions/docs/server2/gcs_file_system.py

Index: chrome/common/extensions/docs/server2/gcs_file_system.py
diff --git a/chrome/common/extensions/docs/server2/gcs_file_system.py b/chrome/common/extensions/docs/server2/gcs_file_system.py
index 11d6c442100d5df9560d97c85de826004a89bb4b..909377ad8046785115e1302104ea07f769397b43 100644
--- a/chrome/common/extensions/docs/server2/gcs_file_system.py
+++ b/chrome/common/extensions/docs/server2/gcs_file_system.py
@@ -9,47 +9,55 @@ from third_party.cloudstorage import errors
 from docs_server_utils import StringIdentity
 from file_system import FileSystem, FileNotFoundError, StatInfo
 from future import Gettable, Future
+from path_util import (
+    AssertIsDirectory, AssertIsFile, AssertIsValid, IsDirectory, Join)
 
 import logging
 import traceback
 
+
+# See gcs_file_system_provider.py for documentation on using Google Cloud
+# Storage as a filesystem.
+#
+# Note that the path requirements for GCS are different for the docserver;
+# GCS requires that paths start with a /, we require that they don't.
+
+
 # Name of the file containing the Git hash of the latest commit sync'ed
 # to Cloud Storage. This file is generated by the Github->GCS sync script
-LAST_COMMIT_HASH_FILENAME='.__lastcommit.txt'
+LAST_COMMIT_HASH_FILENAME = '.__lastcommit.txt'
 
-'''See gcs_file_system_provider.py for documentation on using Google Cloud
-Storage as a filesystem.
-'''
 def _ReadFile(filename):
+  AssertIsFile(filename)
   try:
-    with cloudstorage_api.open(filename, 'r') as f:
+    with cloudstorage_api.open('/' + filename, 'r') as f:
       return f.read()
   except errors.Error:
     raise FileNotFoundError('Read failed for %s: %s' % (filename,
         traceback.format_exc()))
 
 def _ListDir(dir_name):
+  AssertIsDirectory(dir_name)
   try:
-    files = cloudstorage_api.listbucket(dir_name)
-    return [os_path.filename for os_path in files]
+    files = cloudstorage_api.listbucket('/' + dir_name)
+    return [os_path.filename.lstrip('/') for os_path in files]
   except errors.Error:
     raise FileNotFoundError('cloudstorage.listbucket failed for %s: %s' %
                             (dir_name, traceback.format_exc()))
 
 def _CreateStatInfo(bucket, path):
-  bucket = '/%s' % bucket
-  full_path = '/'.join( (bucket, path.lstrip('/')) )
-  last_commit_file = '%s/%s' % (bucket, LAST_COMMIT_HASH_FILENAME)
+  full_path = Join(bucket, path)
+  last_commit_file = Join(bucket, LAST_COMMIT_HASH_FILENAME)
   try:
     last_commit = _ReadFile(last_commit_file)
-    if full_path.endswith('/'):
+    if IsDirectory(full_path):
       child_versions = dict()
       # Fetching stats for all files under full_path, recursively. The
       # listbucket method uses a prefix approach to simulate hierarchy,
       # but calling it without the "delimiter" argument searches for prefix,
       # which means, for directories, everything beneath it.
-      for _file in cloudstorage_api.listbucket(full_path):
-        filename = _file.filename[len(full_path):]
+      for _file in cloudstorage_api.listbucket('/' + full_path):
+        filename = _file.filename.lstrip('/')[len(full_path):]
         child_versions[filename] = last_commit
     else:
       child_versions = None
@@ -71,16 +79,17 @@ class CloudStorageFileSystem(FileSystem):
       logging.debug('gcs: prefixing all bucket names with %s' %
                     debug_bucket_prefix)
       self._bucket = debug_bucket_prefix + self._bucket
+    AssertIsValid(self._bucket)
 
   def Read(self, paths):
     def resolve():
       try:
         result = {}
         for path in paths:
-          full_path = '/%s/%s' % (self._bucket, path.lstrip('/'))
-          logging.debug('gcs: requested path %s, reading %s' %
+          full_path = Join(self._bucket, path)
+          logging.debug('gcs: requested path "%s", reading "%s"' %
                         (path, full_path))
-          if path == '' or path.endswith('/'):
+          if IsDirectory(path):
             result[path] = _ListDir(full_path)
           else:
             result[path] = _ReadFile(full_path)
@@ -95,6 +104,7 @@ class CloudStorageFileSystem(FileSystem):
     return Future(value=())
 
   def Stat(self, path):
+    AssertIsValid(path)
     try:
       return _CreateStatInfo(self._bucket, path)
     except errors.AuthorizationError: