Skip to content

Return an error instead of raising on malformed JSON - #535

Merged
maennchen merged 1 commit into
erlef:mainfrom
ericmj:http-util-invalid-json
Aug 4, 2026
Merged

Return an error instead of raising on malformed JSON#535
maennchen merged 1 commit into
erlef:mainfrom
ericmj:http-util-invalid-json

Conversation

@ericmj

@ericmj ericmj commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

json:decode/1 raises rather than returning an error, and
oidcc_http_util:extract_successful_response/1 called it unguarded in both of its
clauses. A provider that serves a syntactically malformed document under a JSON content
type takes the calling process down instead of returning {error, _}, so every caller
has to wrap oidcc in a try/catch to be safe.

To reproduce, point an adapter at
{ok, {{"HTTP/1.1", 200, ""}, [{"content-type", "application/json"}], <<"{\"issuer\": ">>}}
and call oidcc_provider_configuration:load_configuration/2. It dies with
error:unexpected_end.

The 200/201 clause now returns {error, {invalid_json, Reason}}, a new member of
oidcc_http_util:error(). Every module consuming that union includes it by reference
rather than re-listing its members, so nothing downstream changes.

The non-2xx clause hands back the undecoded body instead. The status code is the real
failure there, and {http_error, Status, Body} already permits a binary body, so a
provider that can't format its own error document no longer masks its own 500. That's
what the unknown content type branch already did.

The catch is narrow on purpose: decode_json/1 is guarded on is_binary(Body). A
non-binary body means the adapter broke its contract, most often by dropping the
body_format option on the way to httpc:request/5, so it keeps crashing with the
stack trace pointing at the adapter instead of being reported as a malformed document.
Reporting it as invalid_json would also put a value outside error() into
{http_error, _, Body} on the non-2xx path.

This covers syntax only. A document that parses but isn't a JSON object, null for
instance, is a separate problem handled where the document is consumed.

One behaviour change: a malformed body used to raise inside telemetry:span/3 and emit
an exception event. It now emits stop with error in the metadata, like every other
error path. test/oidcc_http_adapter_test.erl asserts that.

`json:decode/1` raises rather than returning an error, and
`oidcc_http_util:extract_successful_response/1` called it unguarded in both
clauses. A provider serving a syntactically malformed document under a JSON
content type therefore took the calling process down instead of returning
`{error, _}`, so every caller had to wrap oidcc in a try/catch.

The 200/201 clause now returns `{error, {invalid_json, Reason}}`. The non-2xx
clause hands back the undecoded body, keeping the status code that is the real
failure, exactly as the unknown content type branch already did.

The catch is deliberately narrow. A body that is not a binary means the adapter
broke its contract rather than the provider serving something bad, so it still
crashes with the stack trace pointing at the adapter. Reporting it as a
malformed document would hide the caller's bug, and on the non-2xx path it would
also put a value outside `error()` into `{http_error, _, Body}`.

This covers syntax only. A document that parses but is not a JSON object, `null`
for instance, is a separate problem handled where the document is consumed.
@ericmj
ericmj marked this pull request as ready for review August 4, 2026 03:25
@maennchen
maennchen merged commit 5a39f57 into erlef:main Aug 4, 2026
28 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