From e35286e3322e6fac94fc69dc839f2e15a482eb2f Mon Sep 17 00:00:00 2001
From: "L. Karkkainen" <tronic@users.noreply.github.com>
Date: Sun, 29 Jan 2023 01:43:40 +0000
Subject: [PATCH] Rethinking of renderer selection logic, cleanup.

---
 sanic/errorpages.py | 72 ++++++++++++++++++++-------------------------
 sanic/headers.py    |  8 ++---
 2 files changed, 36 insertions(+), 44 deletions(-)

diff --git a/sanic/errorpages.py b/sanic/errorpages.py
index 5fe72ae8..a1481f0a 100644
--- a/sanic/errorpages.py
+++ b/sanic/errorpages.py
@@ -444,53 +444,45 @@ def exception_response(
     return renderer(request, exception, debug).render()
 
 
-def _guess_renderer(request: Request, fallback: str, base: t.Type[BaseRenderer]) -> t.Type[BaseRenderer]:
-    # base/fallback is app.config.FALLBACK_ERROR_FORMAT
-    render_format = fallback
-    if not request:
-        return base
+def _guess_renderer(req: Request, fallback: str, base: t.Type[BaseRenderer]) -> t.Type[BaseRenderer]:
+    # Renderer selection order:
+    # 1. Accept header (ignoring */* or types with q=0)
+    # 2. Route error_format
+    # 3. FALLBACK if set by app
+    # 4. Content-type for JSON
+    #
+    # If none of the above match or are in conflict with accept header,
+    # then the base renderer is returned.
+    #
+    # Arguments:
+    # - fallback is auto/json/html/text (app.config.FALLBACK_ERROR_FORMAT)
+    # - base is always TextRenderer
 
-    # Try the format from the route via RESPONSE_MAPPING
-    if request.route:
+    # Use the Accept header preference to choose one of the renderers
+    mediatype, accept_q = req.accept.choose(*RENDERERS_BY_CONTENT_TYPE)
+    if accept_q:
+        return RENDERERS_BY_CONTENT_TYPE[mediatype]
+
+    # No clear preference, so employ fuzzy logic to find render_format
+    render_format = fallback
+
+    # Check the route for what the handler returns (magic)
+    # Note: this is done despite having a non-auto fallback
+    if req.route:
         try:
-            if request.route.extra.error_format:
-                render_format = request.route.extra.error_format
+            if req.route.extra.error_format:
+                render_format = req.route.extra.error_format
         except AttributeError:
             pass
 
-    # Do we need to look at the request itself?
+    # If still not known, check for JSON content-type
     if render_format == "auto":
-        # Use the Accept header to choose one
-        mediatype, accept_q = request.accept.choose(*RENDERERS_BY_CONTENT_TYPE)
-        if accept_q:
-            return RENDERERS_BY_CONTENT_TYPE[mediatype]
-        
-        # Otherwise, try JSON content type or request body
-        if not accept_q and "*/*" in request.accept and _check_json_content(request):
-            return JSONRenderer
-        
-        return base
+        mediatype = req.headers.getone("content-type", "").split(";", 1)[0]
+        if mediatype == "application/json":
+            render_format = "json"
 
-    # Use the format from the route if it doesn't contradict the Accept header
+    # Use render_format if found and acceptable, otherwise fallback to base
     renderer = RENDERERS_BY_CONFIG.get(render_format, base)
     type_ = CONTENT_TYPE_BY_RENDERERS[renderer]  # type: ignore
-    acceptable = not request.accept or request.accept.match(type_)
+    acceptable = not req.accept or req.accept.match(type_)
     return renderer if acceptable else base
-
-
-def _check_json_content(request: Request) -> bool:
-    content_type = request.headers.getone("content-type", "").split(";")[0]
-    if content_type == "application/json":
-        return True
-    # Look to see if there was a JSON body
-    # When in this situation, the request is probably coming
-    # from curl, an API client like Postman or Insomnia, or a
-    # package like requests or httpx
-    try:
-        # Give them the benefit of the doubt if they did:
-        # $ curl localhost:8000 -d '{"foo": "bar"}'
-        # And provide them with JSONRenderer
-        if request.json: return True
-    except BadRequest:
-        pass
-    return False
diff --git a/sanic/headers.py b/sanic/headers.py
index b648762e..412f2871 100644
--- a/sanic/headers.py
+++ b/sanic/headers.py
@@ -114,7 +114,7 @@ class MediaType:
 
 class AcceptList(list):
     """A list of media types, as used in the Accept header.
-    
+
     The Accept header entries are listed in order of preference, starting
     with the most preferred. This class is a list of `MediaType` objects,
     that encapsulate also the q value or any other parameters.
@@ -149,7 +149,7 @@ class AcceptList(list):
 
     def choose(self, *media_types: List[str], omit_wildcard=True) -> str:
         """Choose a most suitable media type based on the Accept header.
-        
+
         This is the recommended way to choose a response format based on the
         Accept header. The q values and the order of the Accept header are
         respected, and if due to wildcards multiple arguments match the same
@@ -179,8 +179,8 @@ def parse_accept(accept: str) -> AcceptList:
     https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2
     """
     try:
-        accept_list = [MediaType._parse(mtype) for mtype in accept.split(",")]
-        return AcceptList(sorted(accept_list, key=lambda mtype: -mtype.q))
+        a = [MediaType._parse(mtype) for mtype in accept.split(",") if mtype]
+        return AcceptList(sorted(a, key=lambda mtype: -mtype.q))
     except ValueError:
         raise InvalidHeader(f"Invalid header value in Accept: {accept}")