Repository: pallets/werkzeug (python)

Issue:
have request.get_json() return an error message
Request.get_json(), (or a new function), should return some sort of error message if the `force` parameter is not used notifying the user that this method only functions if the `mime` type is `application/json` unless the `force` option is used.

- in endpoint:
```
parsed_json = request.get_json() # Currently returns None
parsed_json.get("key") # Errors out
```

Alternatively, current usage can look like this:
```
parsed_json = request.get_json() # None
key_value = parsed_json.get("key", None)
if key_value:
    # do stuff
else:
    # do other stuff
```

In most scenarios there would be a failure at some stage or some form of custom handling for the current `None` behavior. Given that the function _expects_ that application/json is in use it seems reasonable to enforce that expectation at some level.

To be totally fair, this would be a breaking change to anyone relying on the current `None` behavior so it would also be great if some sort of `get_json_strict()` existed with similar functionality. The end goal here is to just have some default visibility around this particular behavior of `get_json()`.

This would affect https://github.com/pallets/werkzeug/blob/8292df2c464d9c4b13640f66fb7f393ef20a09a6/src/werkzeug/wrappers/request.py#L540


Pull request:
400 error on bad json content type
`on_json_loading_failed` is now called when the content type isn't `application/json` (and `silent` isn't true), passing `None` instead of an exception. This makes a very common source of confusion (most JavaScript clients don't have special behavior for submitting JSON) visible to the developer.

fixes #2339

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 95c9fa9c..b1cee54c 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -98,6 +98,9 @@ Unreleased
 -   ``Response.autocorrect_location_header`` is disabled by default.
     The ``Location`` header URL will remain relative, and exclude the
     scheme and domain, by default. :issue:`2352`
+-   ``Request.get_json()`` will raise a 400 ``BadRequest`` error if the
+    ``Content-Type`` header is not ``application/json``. This makes a
+    very common source of confusion more visible. :issue:`2339`
 
 
 Version 2.0.3
diff --git a/src/werkzeug/wrappers/request.py b/src/werkzeug/wrappers/request.py
index 475677bc..e44f898a 100644
--- a/src/werkzeug/wrappers/request.py
+++ b/src/werkzeug/wrappers/request.py
@@ -530,6 +530,12 @@ class Request(_SansIORequest):
         (:mimetype:`application/json`, see :attr:`is_json`).
 
         Calls :meth:`get_json` with default arguments.
+
+        If the request content type is not ``application/json``, this
+        will raise a 400 Bad Request error.
+
+        .. versionchanged:: 2.1
+            Raise a 400 error if the content type is incorrect.
         """
         return self.get_json()
 
@@ -543,23 +549,28 @@ class Request(_SansIORequest):
         """Parse :attr:`data` as JSON.
 
         If the mimetype does not indicate JSON
-        (:mimetype:`application/json`, see :attr:`is_json`), this
-        returns ``None``.
-
-        If parsing fails, :meth:`on_json_loading_failed` is called and
-        its return value is used as the return value.
+        (:mimetype:`application/json`, see :attr:`is_json`), or parsing
+        fails, :meth:`on_json_loading_failed` is called and
+        its return value is used as the return value. By default this
+        raises a 400 Bad Request error.
 
         :param force: Ignore the mimetype and always try to parse JSON.
-        :param silent: Silence parsing errors and return ``None``
-            instead.
+        :param silent: Silence mimetype and parsing errors, and
+            return ``None`` instead.
         :param cache: Store the parsed JSON to return for subsequent
             calls.
+
+        .. versionchanged:: 2.1
+            Raise a 400 error if the content type is incorrect.
         """
         if cache and self._cached_json[silent] is not Ellipsis:
             return self._cached_json[silent]
 
         if not (force or self.is_json):
-            return None
+            if not silent:
+                return self.on_json_loading_failed(None)
+            else:
+                return None
 
         data = self.get_data(cache=cache)
 
@@ -584,10 +595,20 @@ class Request(_SansIORequest):
 
         return rv
 
-    def on_json_loading_failed(self, e: ValueError) -> t.Any:
-        """Called if :meth:`get_json` parsing fails and isn't silenced.
+    def on_json_loading_failed(self, e: t.Optional[ValueError]) -> t.Any:
+        """Called if :meth:`get_json` fails and isn't silenced.
+
         If this method returns a value, it is used as the return value
         for :meth:`get_json`. The default implementation raises
         :exc:`~werkzeug.exceptions.BadRequest`.
+
+        :param e: If parsing failed, this is the exception. It will be
+            ``None`` if the content type wasn't ``application/json``.
         """
-        raise BadRequest(f"Failed to decode JSON object: {e}")
+        if e is not None:
+            raise BadRequest(f"Failed to decode JSON object: {e}")
+
+        raise BadRequest(
+            "Did not attempt to load JSON data because the request"
+            " Content-Type was not 'application/json'."
+        )


Test patch:
diff --git a/tests/test_wrappers.py b/tests/test_wrappers.py
index 86fdf876..b769a388 100644
--- a/tests/test_wrappers.py
+++ b/tests/test_wrappers.py
@@ -1346,13 +1346,17 @@ class TestJSON:
         )
         assert response.json == value
 
-    def test_force(self):
+    def test_bad_content_type(self):
         value = [1, 2, 3]
         request = wrappers.Request.from_values(json=value, content_type="text/plain")
-        assert request.json is None
+
+        with pytest.raises(BadRequest):
+            request.get_json()
+
+        assert request.get_json(silent=True) is None
         assert request.get_json(force=True) == value
 
-    def test_silent(self):
+    def test_bad_data(self):
         request = wrappers.Request.from_values(
             data=b'{"a":}', content_type="application/json"
         )