Merge pull request #2665 from fprocess/2651-get_json's-silent-option-weird-side-effect

Fix the bug of cache function in `get_json` method.

CHANGES.rst

@@ -145,6 +145,8 @@ unreleased
   (`#2635`_)
 - A single trailing slash is stripped from the blueprint ``url_prefix``
   when it is registered with the app. (`#2629`_)
+- :meth:`Request.get_json() <flask.Request.get_json>` doesn't cache the
+  result if parsing fails when ``silent`` is true. (`#2651`_)
 
 .. _pallets/meta#24: https://github.com/pallets/meta/issues/24
 .. _#1421: https://github.com/pallets/flask/issues/1421
@@ -189,6 +191,7 @@ unreleased
 .. _#2636: https://github.com/pallets/flask/pull/2636
 .. _#2635: https://github.com/pallets/flask/pull/2635
 .. _#2629: https://github.com/pallets/flask/pull/2629
+.. _#2651: https://github.com/pallets/flask/issues/2651
 
 
 Version 0.12.2

flask/wrappers.py

@@ -23,7 +23,7 @@ class JSONMixin(object):
     .. versionadded:: 1.0
     """
 
-    _cached_json = Ellipsis
+    _cached_json = (Ellipsis, Ellipsis)
 
     @property
     def is_json(self):
@@ -60,8 +60,8 @@ class JSONMixin(object):
         :param silent: Silence parsing errors and return ``None`` instead.
         :param cache: Store the parsed JSON to return for subsequent calls.
         """
-        if cache and self._cached_json is not Ellipsis:
-            return self._cached_json
+        if cache and self._cached_json[silent] is not Ellipsis:
+            return self._cached_json[silent]
 
         if not (force or self.is_json):
             return None
@@ -77,11 +77,17 @@ class JSONMixin(object):
         except ValueError as e:
             if silent:
                 rv = None
+                if cache:
+                    normal_rv, _ = self._cached_json
+                    self._cached_json = (normal_rv, rv)
             else:
                 rv = self.on_json_loading_failed(e)
-
-        if cache:
-            self._cached_json = rv
+                if cache:
+                    _, silent_rv = self._cached_json
+                    self._cached_json = (rv, silent_rv)
+        else:
+            if cache:
+                self._cached_json = (rv, rv)
 
         return rv
 

tests/test_helpers.py

@@ -62,6 +62,21 @@ class TestJSON(object):
             with pytest.raises(BadRequest):
                 flask.request.get_json(silent=False, cache=False)
 
+    def test_different_silent_on_bad_request(self, app):
+        with app.test_request_context(
+                '/', method='POST', data='malformed',
+                content_type='application/json'):
+            assert flask.request.get_json(silent=True) is None
+            with pytest.raises(BadRequest):
+                flask.request.get_json(silent=False)
+
+    def test_different_silent_on_normal_request(self, app):
+        with app.test_request_context('/', method='POST', json={'foo': 'bar'}):
+            silent_rv = flask.request.get_json(silent=True)
+            normal_rv = flask.request.get_json(silent=False)
+            assert silent_rv is normal_rv
+            assert normal_rv['foo'] == 'bar'
+
     def test_post_empty_json_adds_exception_to_response_content_in_debug(self, app, client):
         app.config['DEBUG'] = True
         app.config['TRAP_BAD_REQUEST_ERRORS'] = False