Skip to content

Update to refined content-type spec - #464

Merged
anuraaga merged 3 commits into
connectrpc:mainfrom
anuraaga:content-type-checks
Nov 11, 2026
Merged

anuraaga merged 3 commits into
connectrpc:mainfrom
anuraaga:content-type-checks

Conversation

@anuraaga

Copy link
Copy Markdown
Collaborator

Ignores content-type parameters except for JSON which must be utf-8

connectrpc/connectrpc.com#447
connectrpc/conformance#1191

Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>

@stefanvanburen stefanvanburen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me; matches connectrpc/connectrpc.com#447 as far as I can tell. A few small things, none blocking:

  • validate_response and validate_stream_response parse the content type and then codec_name_from_content_type parses it again; maybe just have one return both?
  • The return "" for "JSON with a bad charset" only works because self._codecs.get("") misses and we 415; might be worth a comment (or None) so nobody accidentally registers a codec under "" 😅.
  • The new client tests live in test/test_client_shared.py, which otherwise tests _client_shared — not sure if there's a better home for ConnectClientProtocol tests. And test_proto_charset_ignored doesn't seem to have an async twin like its neighbors.

(FWIW, _parse_content_type splits on ; without respecting quotes, so something like foo="a;charset=latin1" would 415. Pretty contrived, and conformance doesn't cover it, so probably fine to leave.)

Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
@anuraaga

Copy link
Copy Markdown
Collaborator Author

The return "" for "JSON with a bad charset" only works because self._codecs.get("") misses and we 415; might be worth a comment (or None) so nobody accidentally registers a codec under "" 😅.

Don't think that'll happen so leaving it ;)

@anuraaga
anuraaga merged commit 56123e7 into connectrpc:main Nov 11, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants