general: don't flatten to invalid keys (#52579)

tests/backoffice_pages/test_all.py

@@ -4447,6 +4447,16 @@ def test_inspect_page(pub, local_user):
     assert 'form_user_f3' not in resp.text
     assert 'form_user_f_' not in resp.text
 
+    # make sure workflow data itself is not displayed, as it's available in
+    # expanded variables
+    assert not pq('[title="form_workflow_data"]')
+
+    # but do display it if it has some invalid contents
+    formdata.workflow_data['invalid key'] = 'foobar'
+    formdata.store()
+    resp = login(get_app(pub)).get(resp.request.url)
+    assert resp.pyquery('[title="form_workflow_data"]')
+
     # check functions
     assert re.findall('Recipient.*foobar', resp.text)
 

tests/test_formdata.py

@@ -546,6 +546,24 @@ def test_workflow_data_file_url(pub, formdef):
     assert substvars['foo_var_file_url']
 
 
+def test_workflow_data_invalid_keys(pub, formdef):
+    formdata = formdef.data_class()()
+    formdata.store()
+    formdata.workflow_data = {
+        'valid_key': {'invalid key': 'foo', 'valid_key': 'bar'},
+        'invalid key': 'baz',
+    }
+    substvars = formdata.get_substitution_variables()
+    assert 'form_workflow_data_valid_key' in substvars
+    assert 'form_workflow_data_invalid key' not in substvars
+    assert 'form_workflow_data_valid_key_valid_key' in substvars
+    assert 'form_workflow_data_valid_key_invalid key' not in substvars
+    assert substvars['form_workflow_data_valid_key_valid_key'] == 'bar'
+    with pytest.raises(KeyError):
+        # noqa pylint: disable=pointless-statement
+        substvars['form_workflow_data_invalid key']
+
+
 def test_evolution_get_status(pub):
     Workflow.wipe()
     workflow = Workflow(name='test')

wcs/backoffice/management.py

@@ -3279,10 +3279,16 @@ class FormBackOfficeStatusPage(FormStatusPage):
                     'inspect_url': v['form'].backoffice_url + 'inspect',
                     'display_name': v['form_display_name'],
                 }
-            elif hasattr(v, 'inspect_keys') or isinstance(v, dict):
+            elif hasattr(v, 'inspect_keys'):
-                # skip expanded
+                # skip as there are expanded identifiers
                 continue
             else:
+                if isinstance(v, dict):
+                    # only display dictionaries if they have invalid keys
+                    # (otherwise the expanded identifiers are a better way
+                    # to get to the values).
+                    if all(CompatibilityNamesDict.valid_key_regex.match(k) for k in v):
+                        continue
                 r += htmltext('<li><code title="%s">%s</code>') % (k, k)
                 r += htmltext('  <div class="value"><span>%s</span>') % ellipsize(safe(v), 10000)
                 if not isinstance(v, str):

wcs/formdata.py

@@ -29,7 +29,7 @@ from .qommon import N_, _, misc
 from .qommon.evalutils import make_datetime
 from .qommon.publisher import get_cfg
 from .qommon.storage import Contains, Intersects, Null, StorableObject
-from .qommon.substitution import Substitutions, invalidate_substitution_cache
+from .qommon.substitution import CompatibilityNamesDict, Substitutions, invalidate_substitution_cache
 from .qommon.template import Template
 
 
@@ -908,8 +908,6 @@ class FormData(StorableObject):
     def get_substitution_variables(self, minimal=False):
         from wcs.workflows import AttachmentsSubstitutionProxy
 
-        from .qommon.substitution import CompatibilityNamesDict
-
         variables = CompatibilityNamesDict(
             {
                 'form': self.get_as_lazy(),
@@ -949,7 +947,7 @@ class FormData(StorableObject):
 
             d = copy.deepcopy(d)
             flatten_dict(d)
-            variables.update(d)
+            variables.update({k: v for k, v in d.items() if CompatibilityNamesDict.valid_key_regex.match(k)})
 
         return variables
 

wcs/qommon/substitution.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
+import re
 from contextlib import contextmanager
 
 from quixote import get_publisher
@@ -161,6 +162,9 @@ class Substitutions:
 class CompatibilityNamesDict(dict):
     # custom dictionary that provides automatic fallback to legacy variable
     # names (namespaced with underscores)
+
+    valid_key_regex = re.compile(r'^[a-zA-Z][a-zA-Z0-9_]*$')
+
     def get(self, key, default=None):
         try:
             return self.__getitem__(key)
@@ -178,7 +182,7 @@ class CompatibilityNamesDict(dict):
             if hasattr(item, 'inspect_keys'):
                 sub_keys = item.inspect_keys()
             elif isinstance(item, dict):
-                sub_keys = item.keys()
+                sub_keys = [x for x in item.keys() if self.valid_key_regex.match(x)]
             else:
                 return
             for sub_key in sub_keys:
@@ -226,6 +230,8 @@ class CompatibilityNamesDict(dict):
         raise KeyError
 
     def __getitem__(self, key):
+        if not self.valid_key_regex.match(key):
+            raise KeyError(key)
         parts = key.split('_')
         try:
             return self.get_path(self, parts)