Update to refined content-type spec

src/connectrpc/_protocol_connect.py

@@ -49,26 +49,39 @@
 _DEFAULT_CONNECT_USER_AGENT = f"connectrpc/{__version__}"
 
 
-def _normalize_content_type(content_type: str) -> str:
-    # content-type can have parameters, most commonly charset. Our supported codecs,
-    # binary and JSON are always either non-text or utf-8 and the parameters are not
-    # important for matching to a codec. A custom codec could conceivably need to
-    # match on parameters, but we will reconsider that if it is ever asked for.
-    return content_type.partition(";")[0].strip().lower()
-
-
-def codec_name_from_content_type(content_type: str, *, stream: bool) -> str:
-    content_type = _normalize_content_type(content_type)
+def _charset(params: str) -> str | None:
+    """Return the charset parameter, if any, from content type parameters."""
+    charset = None
+    for param in params.split(";"):
+        name, _, value = param.partition("=")
+        if name.strip().lower() == "charset":
+            value = value.strip()
+            if len(value) >= 2 and value[0] == value[-1] == '"':
+                value = value[1:-1]
+            charset = value.lower()
+    return charset
+
+
+def _parse_content_type(content_type: str, *, stream: bool) -> tuple[str, str]:
+    """Return the normalized media type and the codec name for the content type."""
+    media_type, _, params = content_type.partition(";")
+    media_type = media_type.strip().lower()
     prefix = (
         CONNECT_STREAMING_CONTENT_TYPE_PREFIX
         if stream
         else CONNECT_UNARY_CONTENT_TYPE_PREFIX
     )
-    if content_type.startswith(prefix):
-        return content_type[len(prefix) :]
     # Follow connect-go behavior for malformed content type. If the content type misses the prefix,
     # it will still be coincidentally handled.
-    return content_type
+    codec_name = media_type.removeprefix(prefix)
+    if codec_name == CODEC_NAME_JSON and _charset(params) not in (None, "utf-8"):
+        codec_name = ""
+    return media_type, codec_name
+
+
+def codec_name_from_content_type(content_type: str, *, stream: bool) -> str:
+    """Return the codec name for the content type."""
+    return _parse_content_type(content_type, stream=stream)[1]
 
 
 class ConnectServerProtocol:
@@ -235,22 +248,21 @@ def create_request_context(
     def validate_response(
         self, request_codec_name: str, status_code: int, response_content_type: str
     ) -> None:
-        response_content_type = _normalize_content_type(response_content_type)
+        media_type, response_codec_name = _parse_content_type(
+            response_content_type, stream=False
+        )
         if status_code != HTTPStatus.OK:
             # Error responses must be JSON-encoded
-            if response_content_type == CONNECT_UNARY_CONTENT_TYPE_JSON:
+            if media_type == CONNECT_UNARY_CONTENT_TYPE_JSON:
                 return
             raise ConnectWireError.from_http_status(status_code).to_exception()
 
-        if not response_content_type.startswith(CONNECT_UNARY_CONTENT_TYPE_PREFIX):
+        if not media_type.startswith(CONNECT_UNARY_CONTENT_TYPE_PREFIX):
             raise ConnectError(
                 Code.UNKNOWN,
                 f"invalid content-type: '{response_content_type}'; expecting '{CONNECT_UNARY_CONTENT_TYPE_PREFIX}{request_codec_name}'",
             )
 
-        response_codec_name = codec_name_from_content_type(
-            response_content_type, stream=False
-        )
         if response_codec_name == request_codec_name:
             return
 
@@ -262,15 +274,15 @@ def validate_response(
     def validate_stream_response(
         self, request_codec_name: str, response_content_type: str
     ) -> None:
-        if not response_content_type.startswith(CONNECT_STREAMING_CONTENT_TYPE_PREFIX):
+        media_type, response_codec_name = _parse_content_type(
+            response_content_type, stream=True
+        )
+        if not media_type.startswith(CONNECT_STREAMING_CONTENT_TYPE_PREFIX):
             raise ConnectError(
                 Code.UNKNOWN,
                 f"invalid content-type: '{response_content_type}'; expecting '{CONNECT_STREAMING_CONTENT_TYPE_PREFIX}{request_codec_name}'",
             )
 
-        response_codec_name = response_content_type[
-            len(CONNECT_STREAMING_CONTENT_TYPE_PREFIX) :
-        ]
         if response_codec_name != request_codec_name:
             raise ConnectError(
                 Code.INTERNAL,

test/test_client_shared.py

@@ -2,10 +2,14 @@
 
 from urllib.parse import urlencode
 
+import pytest
 from pyqwest import Headers as HTTPHeaders
 
 from connectrpc._client_shared import prepare_get_params
+from connectrpc._protocol_connect import ConnectClientProtocol
+from connectrpc.code import Code
 from connectrpc.codec import proto_binary_codec
+from connectrpc.errors import ConnectError
 
 
 def test_prepare_get_params_order() -> None:
@@ -27,3 +31,58 @@ def test_prepare_get_params_order() -> None:
 def test_prepare_get_params_order_without_compression() -> None:
     params = prepare_get_params(proto_binary_codec(), b"hello", HTTPHeaders([]))
     assert list(params.keys()) == ["connect", "base64", "encoding", "message"]
+
+
+@pytest.mark.parametrize(
+    ("content_type", "codec_name", "stream"),
+    [
+        ("application/json", "json", False),
+        ("application/json; charset=utf-8", "json", False),
+        ("application/json; charset=UTF-8", "json", False),
+        ('application/json; charset="utf-8"', "json", False),
+        ("application/json; foo=bar; charset=utf-8", "json", False),
+        ("application/json; foo=bar", "json", False),
+        ("application/proto; foo=bar", "proto", False),
+        ("application/proto; charset=iso-8859-1", "proto", False),
+        ("application/connect+json", "json", True),
+        ("application/connect+json; charset=utf-8", "json", True),
+        ("application/connect+json; charset=UTF-8", "json", True),
+        ('application/connect+json; charset="utf-8"', "json", True),
+        ("application/connect+json; foo=bar; charset=utf-8", "json", True),
+        ("application/connect+json; foo=bar", "json", True),
+        ("application/connect+proto; foo=bar", "proto", True),
+    ],
+)
+def test_validate_response_content_type(
+    content_type: str, codec_name: str, *, stream: bool
+) -> None:
+    protocol = ConnectClientProtocol()
+    if stream:
+        protocol.validate_stream_response(codec_name, content_type)
+    else:
+        protocol.validate_response(codec_name, 200, content_type)
+
+
+@pytest.mark.parametrize(
+    ("content_type", "codec_name", "stream", "code"),
+    [
+        ("application/json; charset=iso-8859-1", "json", False, Code.INTERNAL),
+        ("application/json; foo=bar; charset=latin1", "json", False, Code.INTERNAL),
+        ("application/proto", "json", False, Code.INTERNAL),
+        ("text/plain", "json", False, Code.UNKNOWN),
+        ("application/connect+json; charset=latin1", "json", True, Code.INTERNAL),
+        ("application/connect+json; a=b; charset=latin1", "json", True, Code.INTERNAL),
+        ("application/connect+proto", "json", True, Code.INTERNAL),
+        ("application/json", "json", True, Code.UNKNOWN),
+    ],
+)
+def test_validate_response_content_type_invalid(
+    content_type: str, codec_name: str, *, stream: bool, code: Code
+) -> None:
+    protocol = ConnectClientProtocol()
+    with pytest.raises(ConnectError) as exc_info:
+        if stream:
+            protocol.validate_stream_response(codec_name, content_type)
+        else:
+            protocol.validate_response(codec_name, 200, content_type)
+    assert exc_info.value.code == code

test/test_http.py

@@ -17,7 +17,7 @@
 
 from connectrpc._server_sync import _RequestBody
 from connectrpc.code import Code
-from connectrpc.codec import proto_json_codec
+from connectrpc.codec import proto_binary_codec, proto_json_codec
 from connectrpc.errors import ConnectError
 
 from .connectrpc.example.haberdasher_connect import (
@@ -40,6 +40,9 @@
     "application/json;charset=utf-8",
     "application/json;    charset=utf-8",
     "application/json; charset=utf-8; version=1",
+    'application/json; charset="utf-8"',
+    "application/json; version=1",
+    "application/json; version=1; charset=utf-8",
     "application/JSON",
 ]
 
@@ -153,6 +156,83 @@ async def execute(self, request: Request) -> Response:
     assert hats == [Hat(size=2), Hat(size=3)]
 
 
+_unsupported_content_type_cases = [
+    "application/json; charset=iso-8859-1",
+    "application/json; version=1; charset=iso-8859-1",
+    "application/connect+json; charset=iso-8859-1",
+]
+
+
+@pytest.mark.parametrize("header", _unsupported_content_type_cases)
+def test_json_unsupported_charset(header: str) -> None:
+    class NoopHaberdasherSync(HaberdasherSync):
+        pass
+
+    transport = WSGITransport(HaberdasherWSGIApplication(NoopHaberdasherSync()))
+    client = SyncClient(transport=transport)
+
+    method = "MakeSimilarHats" if "connect+" in header else "MakeHat"
+    res = client.post(
+        f"http://localhost/connectrpc.example.Haberdasher/{method}",
+        content=b"{}",
+        headers={"content-type": header},
+    )
+    assert res.status == 415
+
+
+@pytest.mark.anyio
+@pytest.mark.parametrize("header", _unsupported_content_type_cases)
+async def test_json_unsupported_charset_async(header: str) -> None:
+    class NoopHaberdasher(Haberdasher):
+        pass
+
+    transport = ASGITransport(HaberdasherASGIApplication(NoopHaberdasher()))
+    client = Client(transport=transport)
+
+    method = "MakeSimilarHats" if "connect+" in header else "MakeHat"
+    res = await client.post(
+        f"http://localhost/connectrpc.example.Haberdasher/{method}",
+        content=b"{}",
+        headers={"content-type": header},
+    )
+    assert res.status == 415
+
+
+def test_proto_charset_ignored() -> None:
+    class HeadersHaberdasherSync(HaberdasherSync):
+        def make_hat(self, _request, _ctx):
+            return Hat(size=2)
+
+    transport = WSGITransport(HaberdasherWSGIApplication(HeadersHaberdasherSync()))
+    client = SyncClient(transport=transport)
+
+    res = client.post(
+        "http://localhost/connectrpc.example.Haberdasher/MakeHat",
+        content=b"",
+        headers={"content-type": "application/proto; charset=iso-8859-1"},
+    )
+    assert res.status == 200
+    assert proto_binary_codec().decode(res.content, Hat) == Hat(size=2)
+
+
+@pytest.mark.anyio
+async def test_proto_charset_ignored_async() -> None:
+    class HeadersHaberdasher(Haberdasher):
+        async def make_hat(self, _request, _ctx):
+            return Hat(size=2)
+
+    transport = ASGITransport(HaberdasherASGIApplication(HeadersHaberdasher()))
+    client = Client(transport=transport)
+
+    res = await client.post(
+        "http://localhost/connectrpc.example.Haberdasher/MakeHat",
+        content=b"",
+        headers={"content-type": "application/proto; charset=iso-8859-1"},
+    )
+    assert res.status == 200
+    assert proto_binary_codec().decode(res.content, Hat) == Hat(size=2)
+
+
 def test_error_response_does_not_read_past_content_length() -> None:
     body = b"{}"