Whitelist allowed test names

recipe_engine/recipe_test_api.py

 # Copyright 2016 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 import contextlib
 import inspect
+import re
 
 from collections import namedtuple, defaultdict
 
 from .util import ModuleInjectionSite, static_call, static_wraps
 from .types import freeze
 
 def combineify(name, dest, a, b, overwrite=False):
   """
   Combines dictionary members in two objects into a third one using addition.
 
@@ skipping 162 matching lines @@
     ret.update(other)
     return ret
 
   def __repr__(self):
     return "ModuleTestData(%r)" % super(ModuleTestData, self).__repr__()
 
 
 PostprocessHook = namedtuple(
   'PostprocessHook', 'func args kwargs filename lineno')
 
+def is_valid_test_name(name):
+  # * is there to make the match greedy, so it matches the whole word, rather

[Paweł Hajdan Jr., 2017/02/21 09:53:31]

nit: No need to explain regex syntax IMO.

+  # than just the first character of a name
+  match = re.match(r'(\w)*', name)

[dnj, 2017/02/18 00:15:29]

Why not: "name.isalnum()"? Also will reject whitespaces.

[dnj, 2017/02/18 01:27:41]

Ah boo :(

Could do:
re.match(r'[a-zA-Z0-9_]*')

I really just don't like the whitespace check at the end.

[Paweł Hajdan Jr., 2017/02/21 09:53:31]

Let's use ^, $, and +, i.e.

r'^(\w)+$'

Then we don't need check at the end, and we also don't match an empty string:

%>>> re.match(r'(\w)*', ' ')
%<_sre.SRE_Match object at 0x101c92030>
%>>> re.match(r'^(\w)+$', ' ')
%>>>

%>>> re.match(r'(\w)*', '')
%<_sre.SRE_Match object at 0x101c8fb70>
%>>> re.match(r'^(\w)+$', '')
%>>>

+  if not match:
+    return False
+
+  # Catches a case of a name with a space in it.
+  return len(match.group(0)) == len(name)
+
 
 class TestData(BaseTestData):
   def __init__(self, name=None):
     super(TestData, self).__init__()
+    # Allow empty names; sometimes we create test datas to merge two test datas
+    # together.
+    if name and not is_valid_test_name(name):
+      raise ValueError("Invalid test name %r" % name)

[Paweł Hajdan Jr., 2017/02/21 09:53:31]

nit: Single quotes ' instead of double "

Also, explain what valid is (alphanumeric and underscores only).

+
     self.name = name
     self.properties = {}  # key -> val
     self.mod_data = defaultdict(ModuleTestData)
     self.step_data = defaultdict(StepTestData)
     self.depend_on_data = {}
     self.expected_exception = None
     self.post_process_hooks = [] # list(PostprocessHook)
 
   def __add__(self, other):
     assert isinstance(other, TestData)
@@ skipping 443 matching lines @@
         )
     """
     ret = TestData()
     try:
       stk = inspect.stack()
       _, filename, lineno, _, _, _ = stk[1]
     finally:
       del stk
     ret.post_process(func, args, kwargs, filename, lineno)
     return ret