From 2dd72bee52e43b788115992e62c3799e135e32e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Fri, 12 Oct 2018 09:15:52 +0200
Subject: [PATCH] forms: don't let conditional pages alter evaluation of field
 visibility (#27247)

---
 tests/test_form_pages.py   | 44 ++++++++++++++++++++++++++++++++++++++
 wcs/forms/root.py          | 15 ++++++++-----
 wcs/qommon/substitution.py |  8 +++++++
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py
index 0ba637cd1..dceaf9cda 100644
--- a/tests/test_form_pages.py
+++ b/tests/test_form_pages.py
@@ -5298,6 +5298,50 @@ def test_field_live_condition(pub):
     assert '<span class="label">Bar</span>' in resp.body
     assert '<span class="label">Foo</span>' in resp.body
 
+def test_field_condition_on_required_field(pub):
+    # from https://dev.entrouvert.org/issues/27247
+    FormDef.wipe()
+    formdef = FormDef()
+    formdef.name = 'Foo'
+    formdef.fields = [
+        fields.PageField(id='0', label='1st page', type='page'),
+        fields.ItemField(type='item', id='1', label='Bar',
+            items=['oui', 'non'],
+            show_as_radio=True,
+            required=True, varname='bar'),
+        fields.ItemField(type='item', id='2', label='Foo', size='40',
+            required=True,
+            hint='---',
+            items=['plop'],
+            condition={'type': 'django', 'value': 'form_var_bar == "oui"'}),
+        fields.PageField(id='3', label='1st page', type='page',
+            condition={'type': 'python', 'value': 'True'}),
+        fields.CommentField(type='comment', id='4', label='HELLO!'),
+    ]
+    formdef.store()
+
+    app = get_app(pub)
+    resp = app.get('/foo/')
+    assert 'f1' in resp.form.fields
+    assert 'f2' in resp.form.fields
+    assert resp.html.find('div', {'data-field-id': '1'}).attrs['data-live-source'] == 'true'
+    assert resp.html.find('div', {'data-field-id': '2'}).attrs.get('style') == 'display: none'
+    resp.form['f1'] = 'non'
+    live_resp = app.post('/foo/live', params=resp.form.submit_fields())
+    assert live_resp.json['result']['1']['visible']
+    assert not live_resp.json['result']['2']['visible']
+    resp = resp.form.submit('submit')
+    assert 'HELLO' in resp.body
+    resp = resp.form.submit('previous')
+    resp.form['f1'] = 'oui'
+    live_resp = app.post('/foo/live', params=resp.form.submit_fields())
+    assert live_resp.json['result']['1']['visible']
+    assert live_resp.json['result']['2']['visible']
+    print 'HELLO!!!!'
+    resp = resp.form.submit('submit')
+    assert '<div class="error">required field</div>' in resp.body
+    assert 'HELLO' not in resp.body
+
 def test_field_live_condition_multipages(pub):
     FormDef.wipe()
     formdef = FormDef()
diff --git a/wcs/forms/root.py b/wcs/forms/root.py
index 8a51c5956..4d3c4f991 100644
--- a/wcs/forms/root.py
+++ b/wcs/forms/root.py
@@ -524,11 +524,16 @@ class FormPage(Directory, FormTemplateMixin):
         current_data = self.get_transient_formdata().data
         pages = []
         field_page = None
-        for field in self.formdef.fields:
-            if field.type == 'page':
-                field_page = field
-                if field.is_visible(current_data, self.formdef):
-                    pages.append(field)
+        with get_publisher().substitutions.freeze():
+            # don't let evaluation of pages alter substitution variables (this
+            # avoids a ConditionVars being added with current form data and
+            # influencing later code evaluating field visibility based on
+            # submitted data) (#27247).
+            for field in self.formdef.fields:
+                if field.type == 'page':
+                    field_page = field
+                    if field.is_visible(current_data, self.formdef):
+                        pages.append(field)
         if not field_page:  # form without page fields
             pages = [None]
         self._pages = pages
diff --git a/wcs/qommon/substitution.py b/wcs/qommon/substitution.py
index 36514afd5..0ee48a013 100644
--- a/wcs/qommon/substitution.py
+++ b/wcs/qommon/substitution.py
@@ -69,6 +69,14 @@ class Substitutions(object):
             self.sources.append(source)
             self.invalidate_cache()
 
+    @contextmanager
+    def freeze(self):
+        orig_sources, self.sources = self.sources, self.sources[:]
+        self.invalidate_cache()
+        yield
+        self.sources = orig_sources
+        self.invalidate_cache()
+
     @contextmanager
     def temporary_feed(self, source, force_mode=None):
         if source is None or source in self.sources:
-- 
2.19.1