Repository: sanic-org/sanic

Issue:
HTTP 1 request headers decoded using default encoding instead of ISO-8859-1
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

headers are decoded here without specifying their encoding:

https://github.com/sanic-org/sanic/blob/ad4e526c775fc3ce950503d6476d9d344492b0dd/sanic/http/http1.py#L205

On my system (osx using python 3.10.8 installed via homebrew) this causes bytes that are valid characters in ISO-8859-1 but not in UTF-8 to be decoded as surrogate escape characters, e.g. `b"\x80"` becomes `"\udf80"` instead of `"\x80"`

### Code snippet

_No response_

### Expected Behavior

headers encoded as ISO-8859-1 with no MIME type to be decoded correctly without using UTF-8 surrogate escape characters.

### How do you run Sanic?

As a script (`app.run` or `Sanic.serve`)

### Operating System

linux

### Sanic Version

22.9.1

### Additional context

this used to work as expected in Sanic<=20.12.7

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/sanic-org/sanic/pull/2606?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org) Report
Base: **88.047**% // Head: **88.047**% // No change to project coverage :thumbsup:
> Coverage data is based on head [(`7bbd725`)](https://codecov.io/gh/sanic-org/sanic/pull/2606?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org) compared to base [(`beae35f`)](https://codecov.io/gh/sanic-org/sanic/commit/beae35f921f170c6aba54bd418103ad86f30e21f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org).
> Patch coverage: 100.000% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##              main     #2606   +/-   ##
=========================================
  Coverage   88.047%   88.047%           
=========================================
  Files           71        71           
  Lines         5338      5338           
  Branches       893       893           
=========================================
  Hits          4700      4700           
  Misses         461       461           
  Partials       177       177           
```


| [Impacted Files](https://codecov.io/gh/sanic-org/sanic/pull/2606?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org) | Coverage Δ | |
|---|---|---|
| [sanic/http/http1.py](https://codecov.io/gh/sanic-org/sanic/pull/2606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org#diff-c2FuaWMvaHR0cC9odHRwMS5weQ==) | `84.827% <ø> (ø)` | |
| [sanic/http/http3.py](https://codecov.io/gh/sanic-org/sanic/pull/2606/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org#diff-c2FuaWMvaHR0cC9odHRwMy5weQ==) | `80.555% <100.000%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/sanic-org/sanic/pull/2606?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sanic-org).

Tronic: This should probably continued. Needs some practical testing with ASGI servers to see how they behave, but the likely outcome is that we need to use UTF-8 on that side as well, the same as with the integrated server.

Two points of interest:
- Header encoding (values in particular, keys should stay ASCII).
- Path encoding, and with that also how clients handle it.

I can help run those tests once I get my other PRs sorted out, but @ChihweiLHBird feel free to do your own tests if you can. If it looks like that UTF-8 is going to work identically with ASGI as it does with the Sanic server, we are all good to implement UTF-8 on ASGI as well.
ChihweiLHBird: @Tronic Sure, I can try to test it.
Tronic: @ChihweiLHBird Could you get this finished? We had a discussion and want to have this in the upcoming 23.3 release.

ChihweiLHBird: @Tronic Sorry, I was busy last few days. I am on it right now.
ChihweiLHBird: @Tronic Just done some manual testing, and it seems for the test cases I tried, there is not an issue for header decoding. Like if it uses UTF-8, it will successfully decode emojis and other UTF-8 characters but failed to decode some characters in latin-1 which isn't in UTF-8, like `b"\x80"`. Similar result to path. And I think this is the expected results, right? I am not sure what are the more comprehensive test cases I should try. 
Tronic: Note on further changes: (not necessarily in this PR)
- http1 needs to use ASCII for url_bytes encoding too (and raise that same exception)
- http3 alike needs to encode in ASCII, and also needs ASCII/surrogateescape field name/value as here.

After these changes all three modes should behave more closely to each other, and be stricter in RFC compliance.

Tronic: PR #2710 now implements charsets in URL handling, so that should not be addressed here. But implement error handling for headers as instructed above.

ChihweiLHBird: > PR #2710 now implements charsets in URL handling, so that should not be addressed here. But implement error handling for headers as instructed above.

What about http1 headers?
https://github.com/sanic-org/sanic/blob/88c918e72f496ed22405b8e3a0fff672553ea2ff/sanic/http/http1.py#L205
Tronic: > What about http1 headers?

It will stay laxer with header names. The entire header is decoded at once because that is faster than decoding each field separately, and thus we cannot check that they are ASCII (without performance penalty).

ChihweiLHBird: @ahopkins May I have your review?

Patch:
diff --git a/sanic/asgi.py b/sanic/asgi.py
index e5c07c2a..27285c35 100644
--- a/sanic/asgi.py
+++ b/sanic/asgi.py
@@ -6,7 +6,7 @@ from typing import TYPE_CHECKING, Optional
 from urllib.parse import quote
 
 from sanic.compat import Header
-from sanic.exceptions import ServerError
+from sanic.exceptions import BadRequest, ServerError
 from sanic.helpers import Default
 from sanic.http import Stage
 from sanic.log import error_logger, logger
@@ -132,12 +132,20 @@ class ASGIApp:
         instance.sanic_app.state.is_started = True
         setattr(instance.transport, "add_task", sanic_app.loop.create_task)
 
-        headers = Header(
-            [
-                (key.decode("latin-1"), value.decode("latin-1"))
-                for key, value in scope.get("headers", [])
-            ]
-        )
+        try:
+            headers = Header(
+                [
+                    (
+                        key.decode("ASCII"),
+                        value.decode(errors="surrogateescape"),
+                    )
+                    for key, value in scope.get("headers", [])
+                ]
+            )
+        except UnicodeDecodeError:
+            raise BadRequest(
+                "Header names can only contain US-ASCII characters"
+            )
         path = (
             scope["path"][1:]
             if scope["path"].startswith("/")


Test patch:
diff --git a/tests/test_asgi.py b/tests/test_asgi.py
index 6b6872ac..af44ead3 100644
--- a/tests/test_asgi.py
+++ b/tests/test_asgi.py
@@ -7,6 +7,9 @@ from unittest.mock import call
 import pytest
 import uvicorn
 
+from httpx import Headers
+from pytest import MonkeyPatch
+
 from sanic import Sanic
 from sanic.application.state import Mode
 from sanic.asgi import ASGIApp, Lifespan, MockTransport
@@ -626,3 +629,26 @@ async def test_error_on_lifespan_exception_stop(app: Sanic):
             )
         ]
     )
+
+
+@pytest.mark.asyncio
+async def test_asgi_headers_decoding(app: Sanic, monkeypatch: MonkeyPatch):
+    @app.get("/")
+    def handler(request: Request):
+        return text("")
+
+    headers_init = Headers.__init__
+
+    def mocked_headers_init(self, *args, **kwargs):
+        if "encoding" in kwargs:
+            kwargs.pop("encoding")
+        headers_init(self, encoding="utf-8", *args, **kwargs)
+
+    monkeypatch.setattr(Headers, "__init__", mocked_headers_init)
+
+    message = "Header names can only contain US-ASCII characters"
+    with pytest.raises(BadRequest, match=message):
+        _, response = await app.asgi_client.get("/", headers={"😂": "😅"})
+
+    _, response = await app.asgi_client.get("/", headers={"Test-Header": "😅"})
+    assert response.status_code == 200