From a29c7892a747874d0a72e7df330216f11c393508 Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Sun, 6 Oct 2024 10:15:06 +0200 Subject: [PATCH] feat(resp): add option to disable XML serialization of errors (#2356) * feat: add option to enable xml serialization on errors. The option is on by default * chore: add coverage to missing branch * test: fix failing test * chore: review feedback * docs: improve documentation --- docs/_newsfragments/2355.newandimproved.rst | 10 +++ falcon/app_helpers.py | 19 +++-- falcon/http_error.py | 25 ++++--- falcon/response.py | 20 +++++- tests/test_error_handlers.py | 1 + tests/test_httperror.py | 78 +++++++++++++++++++-- 6 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 docs/_newsfragments/2355.newandimproved.rst diff --git a/docs/_newsfragments/2355.newandimproved.rst b/docs/_newsfragments/2355.newandimproved.rst new file mode 100644 index 000000000..8cbc812ec --- /dev/null +++ b/docs/_newsfragments/2355.newandimproved.rst @@ -0,0 +1,10 @@ +Added a new flag :attr:`~falcon.ResponseOptions.xml_error_serialization` that +can be used to disable XML serialization of :class:`~falcon.HTTPError` when +using the default error serializer (and the client preferred it). +This flag currently defaults to ``True``, keeping the same behavior as the previous +Falcon series; Falcon 5.0 will change the default to ``False``, with the objective of +completely removing support for it in a future version. +User that whish to retain support for XML serialization of the errors in the default +error serializer should add an XML media handler. + +Deprecated the :meth:`~falcon.HTTPError.to_xml` method. diff --git a/falcon/app_helpers.py b/falcon/app_helpers.py index d83731005..68ccc28d9 100644 --- a/falcon/app_helpers.py +++ b/falcon/app_helpers.py @@ -291,9 +291,13 @@ def default_serialize_error(req: Request, resp: Response, exception: HTTPError) resp: Instance of ``falcon.Response`` exception: Instance of ``falcon.HTTPError`` """ - - predefined = [MEDIA_JSON, 'text/xml', MEDIA_XML] - media_handlers = [mt for mt in resp.options.media_handlers if mt not in predefined] + options = resp.options + predefined = ( + [MEDIA_JSON, 'text/xml', MEDIA_XML] + if options.xml_error_serialization + else [MEDIA_JSON] + ) + media_handlers = [mt for mt in options.media_handlers if mt not in predefined] # NOTE(caselit,vytas): Add the registered handlers after the predefined # ones. This ensures that in the case of an equal match, the first one # (JSON) is selected and that the q parameter is taken into consideration @@ -317,10 +321,13 @@ def default_serialize_error(req: Request, resp: Response, exception: HTTPError) if '+json' in accept: preferred = MEDIA_JSON elif '+xml' in accept: + # NOTE(caselit): ignore xml_error_serialization when + # checking if the media should be xml. This gives a chance to + # a xml media handler, if any, to be used preferred = MEDIA_XML if preferred is not None: - handler, _, _ = resp.options.media_handlers._resolve( + handler, _, _ = options.media_handlers._resolve( preferred, MEDIA_JSON, raise_not_found=False ) if preferred == MEDIA_JSON: @@ -333,8 +340,8 @@ def default_serialize_error(req: Request, resp: Response, exception: HTTPError) # to re-get the handler, since async handlers may not have a sync # version available. resp.media = exception.to_dict() - else: - resp.data = exception.to_xml() + elif options.xml_error_serialization: + resp.data = exception._to_xml() # NOTE(kgriffs): No need to append the charset param, since # utf-8 is the default for both JSON and XML. diff --git a/falcon/http_error.py b/falcon/http_error.py index b6c104aec..26a6b18f9 100644 --- a/falcon/http_error.py +++ b/falcon/http_error.py @@ -19,6 +19,7 @@ import xml.etree.ElementTree as et from falcon.constants import MEDIA_JSON +from falcon.util import deprecated from falcon.util import misc from falcon.util import uri @@ -43,8 +44,7 @@ class HTTPError(Exception): To customize what data is passed to the serializer, subclass ``HTTPError`` and override the ``to_dict()`` method (``to_json()`` - is implemented via ``to_dict()``). To also support XML, override - the ``to_xml()`` method. + is implemented via ``to_dict()``). `status` is the only positional argument allowed, the other arguments are defined as keyword-only. @@ -214,13 +214,8 @@ def to_json(self, handler: Optional[BaseHandler] = None) -> bytes: # NOTE: the json handler requires the sync serialize interface return handler.serialize(obj, MEDIA_JSON) - def to_xml(self) -> bytes: - """Return an XML-encoded representation of the error. - - Returns: - bytes: An XML document for the error. - - """ + def _to_xml(self) -> bytes: + """Return an XML-encoded representation of the error.""" error_element = et.Element('error') @@ -242,6 +237,18 @@ def to_xml(self) -> bytes: error_element, encoding='utf-8' ) + @deprecated( + 'The internal serialization to xml is deprecated. A similar output can be ' + 'obtained by serializing to xml the output of ``.to_dict()``' + ) + def to_xml(self) -> bytes: + """Return an XML-encoded representation of the error. + + Returns: + bytes: An XML document for the error. + """ + return self._to_xml() + # NOTE: initialized in falcon.media.json, that is always imported since Request/Response # are imported by falcon init. diff --git a/falcon/response.py b/falcon/response.py index e71a4b4c9..afcc8941a 100644 --- a/falcon/response.py +++ b/falcon/response.py @@ -1387,7 +1387,7 @@ class ResponseOptions: media_handlers: Handlers """A dict-like object for configuring the media-types to handle. - default, handlers are provided for the ``application/json``, + Default handlers are provided for the ``application/json``, ``application/x-www-form-urlencoded`` and ``multipart/form-data`` media types. """ static_media_types: Dict[str, str] @@ -1395,18 +1395,36 @@ class ResponseOptions: Defaults to ``mimetypes.types_map`` after calling ``mimetypes.init()``. """ + xml_error_serialization: bool + """Set to ``False`` to disable automatic inclusion of the XML handler + in the default error serializer (:ref:`errors`) (default ``True``). + + Enabling this option does not automatically render all error response in XML, + but only if the client prefers (via the ``Accept`` request header) XML to JSON + and other configured media handlers. + + Note: + This option will default to ``False`` in Falcon 5.0 disabling XML error + serialization by default). + + Note: + This option has no effect when a custom error serializer, set using + :meth:`~falcon.App.set_error_serializer`, is in use. + """ __slots__ = ( 'secure_cookies_by_default', 'default_media_type', 'media_handlers', 'static_media_types', + 'xml_error_serialization', ) def __init__(self) -> None: self.secure_cookies_by_default = True self.default_media_type = DEFAULT_MEDIA_TYPE self.media_handlers = Handlers() + self.xml_error_serialization = True if not mimetypes.inited: mimetypes.init() diff --git a/tests/test_error_handlers.py b/tests/test_error_handlers.py index 5bac0842a..0fad68920 100644 --- a/tests/test_error_handlers.py +++ b/tests/test_error_handlers.py @@ -77,6 +77,7 @@ def test_caught_error(self, client): def test_uncaught_python_error( self, client, get_headers, resp_content_type, resp_start ): + client.app.resp_options.xml_error_serialization = True result = client.simulate_get(headers=get_headers) assert result.status_code == 500 assert result.headers['content-type'] == resp_content_type diff --git a/tests/test_httperror.py b/tests/test_httperror.py index fb59c0f85..9441f32b3 100644 --- a/tests/test_httperror.py +++ b/tests/test_httperror.py @@ -12,6 +12,7 @@ from falcon.constants import MEDIA_YAML from falcon.media import BaseHandler import falcon.testing as testing +from falcon.util.deprecation import DeprecatedWarning try: import yaml @@ -29,6 +30,23 @@ def client(asgi, util): return testing.TestClient(app) +@pytest.fixture( + params=[ + pytest.param(True, id='with_xml'), + pytest.param(False, id='without_xml'), + pytest.param(None, id='default_xml'), + ] +) +def enable_xml(request): + def go(app): + if request.param is not None: + app.resp_options.xml_error_serialization = request.param + # return bool(request.param) + return request.param is not False + + return go + + class FaultyResource: def on_get(self, req, resp): status = req.get_header('X-Error-Status') @@ -302,9 +320,10 @@ def test_no_description_json(self, client): response = client.simulate_patch('/fail') assert response.status == falcon.HTTP_400 assert response.json == {'title': '400 Bad Request'} - assert response.headers['Content-Type'] == 'application/json' + assert response.content_type == 'application/json' def test_no_description_xml(self, client): + client.app.resp_options.xml_error_serialization = True response = client.simulate_patch( path='/fail', headers={'Accept': 'application/xml'} ) @@ -316,7 +335,35 @@ def test_no_description_xml(self, client): ) assert response.content == expected_xml - assert response.headers['Content-Type'] == 'application/xml' + assert response.content_type == 'application/xml' + + @pytest.mark.parametrize('custom_xml', [True, False]) + def test_xml_enable(self, client, enable_xml, custom_xml): + has_xml = enable_xml(client.app) + client.app.resp_options.default_media_type = 'app/foo' + accept = 'app/falcon+xml' if custom_xml else 'application/xml' + response = client.simulate_patch(path='/fail', headers={'Accept': accept}) + assert response.status == falcon.HTTP_400 + + if has_xml: + expected_xml = ( + b'' + b'400 Bad Request' + ) + assert response.content == expected_xml + else: + assert response.content == b'' + if has_xml or custom_xml: + assert response.content_type == 'application/xml' + else: + assert response.content_type == 'app/foo' + + def test_to_xml_deprecated(self): + with pytest.warns( + DeprecatedWarning, match='The internal serialization to xml is deprecated.' + ): + res = falcon.HTTPGone().to_xml() + assert res == falcon.HTTPGone()._to_xml() def test_client_does_not_accept_json_or_xml(self, client): headers = { @@ -499,6 +546,7 @@ def test_epic_fail_json(self, client): ], ) def test_epic_fail_xml(self, client, media_type): + client.app.resp_options.xml_error_serialization = True headers = {'Accept': media_type} expected_body = ( @@ -547,6 +595,7 @@ def test_unicode_json(self, client): assert expected_body == response.json def test_unicode_xml(self, client): + client.app.resp_options.xml_error_serialization = True unicode_resource = UnicodeFaultyResource() expected_body = ( @@ -738,11 +787,12 @@ def test_414_with_custom_kwargs(self, client): def test_416(self, client, asgi, util): client.app = util.create_app(asgi) + client.app.resp_options.xml_error_serialization = True client.app.add_route('/416', RangeNotSatisfiableResource()) response = client.simulate_request(path='/416', headers={'accept': 'text/xml'}) assert response.status == falcon.HTTP_416 - assert response.content == falcon.HTTPRangeNotSatisfiable(123456).to_xml() + assert response.content == falcon.HTTPRangeNotSatisfiable(123456)._to_xml() exp = ( b'' b'416 Range Not Satisfiable' @@ -998,6 +1048,12 @@ def client(self, util, asgi): app.add_route('/', GoneResource()) return testing.TestClient(app) + def test_unknown_accept(self, client): + res = client.simulate_get(headers={'Accept': 'foo/bar'}) + assert res.content_type == 'application/json' + assert res.headers['vary'] == 'Accept' + assert res.content == b'' + @pytest.mark.parametrize('has_json_handler', [True, False]) def test_defaults_to_json(self, client, has_json_handler): if not has_json_handler: @@ -1015,6 +1071,7 @@ def test_defaults_to_json(self, client, has_json_handler): def test_serializes_error_to_preferred_by_sender( self, accept, content_type, content, client, asgi ): + client.app.resp_options.xml_error_serialization = True client.app.resp_options.media_handlers[MEDIA_YAML] = FakeYamlMediaHandler() client.app.resp_options.media_handlers[ASYNC_WITH_SYNC[0]] = ( SyncInterfaceMediaHandler() @@ -1041,9 +1098,11 @@ def test_json_async_only_error(self, util): with pytest.raises(NotImplementedError, match='requires the sync interface'): client.simulate_get() - def test_add_xml_handler(self, client): + @pytest.mark.parametrize('accept', [MEDIA_XML, 'application/xhtml+xml']) + def test_add_xml_handler(self, client, enable_xml, accept): + enable_xml(client.app) client.app.resp_options.media_handlers[MEDIA_XML] = FakeYamlMediaHandler() - res = client.simulate_get(headers={'Accept': 'application/xhtml+xml'}) + res = client.simulate_get(headers={'Accept': accept}) assert res.content_type == MEDIA_XML assert res.content == YAML[-1] @@ -1067,7 +1126,12 @@ def test_add_xml_handler(self, client): (f'text/html,{MEDIA_YAML};q=0.8,{MEDIA_JSON};q=0.8', MEDIA_JSON), ], ) - def test_hard_content_types(self, client, accept, content_type): + def test_hard_content_types(self, client, accept, content_type, enable_xml): + has_xml = enable_xml(client.app) + client.app.resp_options.default_media_type = 'my_type' client.app.resp_options.media_handlers[MEDIA_YAML] = FakeYamlMediaHandler() res = client.simulate_get(headers={'Accept': accept}) - assert res.content_type == content_type + if has_xml or content_type != MEDIA_XML: + assert res.content_type == content_type + else: + assert res.content_type == MEDIA_JSON