Enforces strict manifest content.

tools/isolate/run_test_from_archive.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Reads a manifest, creates a tree of hardlinks and runs the test.
 
 Keeps a local cache.
 """
 
 import ctypes
 import json
 import logging
 import optparse
 import os
 import re
 import shutil
 import stat
 import subprocess
 import sys
 import tempfile
 import time
 import urllib
 
 
 # Types of action accepted by recreate_tree().
 HARDLINK, SYMLINK, COPY = range(1, 4)
 
+RE_IS_SHA1 = re.compile(r'^[a-fA-F0-9]{40}$')
+
+
+class ConfigError(ValueError):
+  """Generic failure to load a manifest."""
+  pass
+
 
 class MappingError(OSError):
   """Failed to recreate the tree."""
   pass
 
 
 def os_link(source, link_name):
   """Add support for os.link() on Windows."""
   if sys.platform == 'win32':
     if not ctypes.windll.kernel32.CreateHardLinkW(
@@ skipping 126 matching lines @@
 
 
 def make_temp_dir(prefix, root_dir):
   """Returns a temporary directory on the same file system as root_dir."""
   base_temp_dir = None
   if not is_same_filesystem(root_dir, tempfile.gettempdir()):
     base_temp_dir = os.path.dirname(root_dir)
   return tempfile.mkdtemp(prefix=prefix, dir=base_temp_dir)
 
 
+def load_manifest(content):
+  """Verifies the manifest is valid and loads this object with the json data.
+  """
+  data = json.loads(content)
+  if not isinstance(data, dict):
+    raise ConfigError('Expected dict, got %r' % data)
+
+  for key, value in data.iteritems():
+    if key == 'command':
+      if not isinstance(value, list):
+        raise ConfigError('Expected list, got %r' % value)
+      for subvalue in value:
+        if not isinstance(subvalue, basestring):
+          raise ConfigError('Expected string, got %r' % subvalue)
+
+    elif key == 'files':
+      if not isinstance(value, dict):
+        raise ConfigError('Expected dict, got %r' % value)
+      for subkey, subvalue in value.iteritems():
+        if not isinstance(subkey, basestring):
+          raise ConfigError('Expected string, got %r' % subkey)
+        if not isinstance(subvalue, dict):
+          raise ConfigError('Expected dict, got %r' % subvalue)
+        for subsubkey, subsubvalue in subvalue.iteritems():

[M-A Ruel, 2012/08/28 20:51:02]

The main issue is that it's much easier to make a typo due to the way scoping is
done in python.

But yeah, it's shorter.

+          if subsubkey == 'link':
+            if not isinstance(subsubvalue, basestring):
+              raise ConfigError('Expected string, got %r' % subsubvalue)
+          elif subsubkey == 'mode':
+            if not isinstance(subsubvalue, int):
+              raise ConfigError('Expected int, got %r' % subsubvalue)
+          elif subsubkey == 'sha-1':
+            if not RE_IS_SHA1.match(subsubvalue):
+              raise ConfigError('Expected sha-1, got %r' % subsubvalue)
+          elif subsubkey == 'timestamp':
+            if not isinstance(subsubvalue, int):
+              raise ConfigError('Expected int, got %r' % subsubvalue)
+          else:
+            raise ConfigError('Unknown key %s' % subsubkey)
+        if bool('sha-1' in subvalue) and bool('link' in subvalue):
+          raise ConfigError(
+              'Did not expect both \'sha-1\' and \'link\', got: %r' % subvalue)
+
+    elif key == 'read_only':
+      if not isinstance(value, bool):
+        raise ConfigError('Expected bool, got %r' % value)
+
+    elif key == 'relative_cwd':
+      if not isinstance(value, basestring):
+        raise ConfigError('Expected string, got %r' % value)
+
+    else:
+      raise ConfigError('Unknown key %s' % subkey)
+
+  return data
+
+
 def fix_python_path(cmd):
   """Returns the fixed command line to call the right python executable."""
   out = cmd[:]
   if out[0] == 'python':
     out[0] = sys.executable
   elif out[0].endswith('.py'):
     out.insert(0, sys.executable)
   return out
 
 
@@ skipping 202 matching lines @@
             os.makedirs(outfiledir)
           if 'sha-1' in properties:
             # A normal file.
             infile = properties['sha-1']
             cache.retrieve(infile)
             link_file(outfile, cache.path(infile), HARDLINK)
           elif 'link' in properties:
             # A symlink.
             os.symlink(properties['link'], outfile)
           else:
-            raise ValueError('Unexpected entry: %s' % properties)
+            raise ConfigError('Unexpected entry: %s' % properties)
           if 'mode' in properties:
             # It's not set on Windows.
             os.chmod(outfile, properties['mode'])
 
       cwd = os.path.join(outdir, manifest.get('relative_cwd', ''))
       if not os.path.isdir(cwd):
         os.makedirs(cwd)
       if manifest.get('read_only'):
         make_writable(outdir, True)
       cmd = manifest['command']
@@ skipping 68 matching lines @@
     parser.error('One and only one of --manifest or --hash is required.')
   if not options.remote:
     parser.error('--remote is required.')
   if args:
     parser.error('Unsupported args %s' % ' '.join(args))
 
   if options.hash:
     # First calculate the reference to it.
     options.manifest = '%s/%s' % (options.remote.rstrip('/'), options.hash)
   try:
-    manifest = json.load(open_remote(options.manifest))
+    manifest = load_manifest(open_remote(options.manifest).read())
   except IOError as e:
     parser.error(
         'Failed to read manifest %s; remote:%s; hash:%s; %s' %
         (options.manifest, options.remote, options.hash, str(e)))
 
   policies = CachePolicies(
       options.max_cache_size, options.min_free_space, options.max_items)
   try:
     return run_tha_test(
         manifest,
         os.path.abspath(options.cache),
         options.remote,
         policies)
-  except MappingError, e:
+  except (ConfigError, MappingError), e:
     print >> sys.stderr, str(e)
     return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())