Enforces strict manifest content.

This prevents typos and other silly and hard to track errors.
Add new unit test.

R=cmp@chromium.org
NOTRY=true
BUG=


Review URL: https://chromiumcodereview.appspot.com/10880009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@153747 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 125 insertions, 3 deletions

diff --git a/tools/isolate/run_test_from_archive.py b/tools/isolate/run_test_from_archive.py
index d8b2bed..e568899 100755
--- a/tools/isolate/run_test_from_archive.py
+++ b/tools/isolate/run_test_from_archive.py
@@ -26,6 +26,13 @@ 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."""
@@ -172,6 +179,62 @@ def make_temp_dir(prefix, 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():
+          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[:]
@@ -394,7 +457,7 @@ def run_tha_test(manifest, cache_dir, remote, policies):
             # 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'])
@@ -483,7 +546,7 @@ def main():
     # 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' %
@@ -497,7 +560,7 @@ def main():
         os.path.abspath(options.cache),
         options.remote,
         policies)
-  except MappingError, e:
+  except (ConfigError, MappingError), e:
     print >> sys.stderr, str(e)
     return 1
 
diff --git a/tools/isolate/run_test_from_archive_test.py b/tools/isolate/run_test_from_archive_test.py
new file mode 100755
index 0000000..0a6f5c9
--- /dev/null
+++ b/tools/isolate/run_test_from_archive_test.py
@@ -0,0 +1,59 @@
+#!/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.
+
+import json
+import logging
+import sys
+import unittest
+
+import run_test_from_archive
+
+
+class RunTestFromArchiveTest(unittest.TestCase):
+  def test_load_manifest_empty(self):
+    m = run_test_from_archive.load_manifest('{}')
+    self.assertEquals({}, m)
+
+  def test_load_manifest_good(self):
+    data = {
+      u'command': [u'foo', u'bar'],
+      u'files': {
+        u'a': {
+          u'link': u'somewhere',
+          u'mode': 123,
+          u'timestamp': 456,
+        },
+        u'b': {
+          u'mode': 123,
+          u'sha-1': u'0123456789abcdef0123456789abcdef01234567'
+        }
+      },
+      u'read_only': False,
+      u'relative_cwd': u'somewhere_else'
+    }
+    m = run_test_from_archive.load_manifest(json.dumps(data))
+    self.assertEquals(data, m)
+
+  def test_load_manifest_bad(self):
+    data = {
+      u'files': {
+        u'a': {
+          u'link': u'somewhere',
+          u'sha-1': u'0123456789abcdef0123456789abcdef01234567'
+        }
+      },
+    }
+    try:
+      run_test_from_archive.load_manifest(json.dumps(data))
+      self.fail()
+    except run_test_from_archive.ConfigError:
+      pass
+
+
+
+if __name__ == '__main__':
+  logging.basicConfig(
+      level=(logging.DEBUG if '-v' in sys.argv else logging.ERROR))
+  unittest.main()