Skip to content

Return an error instead of throwing on invalid UTF-8 in deserialisation helpers#1249

Open
palas wants to merge 3 commits into
masterfrom
nonthrowing-utf8-decode
Open

Return an error instead of throwing on invalid UTF-8 in deserialisation helpers#1249
palas wants to merge 3 commits into
masterfrom
nonthrowing-utf8-decode

Conversation

@palas

@palas palas commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Context

deserialiseInput, deserialiseInputAnyOf and deserialiseAnyVerificationKeyBech32 (and thereby deserialiseAnyVerificationKey) used Text.decodeUtf8, which throws an impure exception when the input is not valid UTF-8. For example, deserialiseInput on the bytes "\xc3\x28" threw instead of returning Left InputInvalidError.

This PR switches them to Text.decodeUtf8' and treats invalid UTF-8 as a normal decoding failure.

How to trust this PR

  • Invalid UTF-8 can never be valid Bech32, so mapping the decode failure to the existing "not valid Bech32" path (DeserialiseInputErrorFormatMismatch / Bech32DecodingError) does not change behaviour for any valid input.
  • The three changed sites are mechanical: wrap the old code in the Right case of decodeUtf8'.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff
  • Changelog fragment added in .changes/

Copilot AI review requested due to automatic review settings July 8, 2026 23:10
@palas palas self-assigned this Jul 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens Cardano API deserialisation helpers by avoiding Text.decodeUtf8’s impure exception on invalid UTF-8, ensuring these helpers return normal decoding errors instead of crashing.

Changes:

  • Switched Bech32 deserialisation paths in deserialiseInput / deserialiseInputAnyOf to use Text.decodeUtf8' and treat invalid UTF-8 as a format mismatch.
  • Updated deserialiseAnyVerificationKeyBech32 to avoid throwing on invalid UTF-8 and return a decoding error instead.
  • Added a Herald changelog fragment documenting the bugfix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
cardano-api/src/Cardano/Api/Serialise/DeserialiseAnyOf.hs Avoids impure exceptions by using decodeUtf8' and mapping invalid UTF-8 to Bech32 format mismatch.
cardano-api/src/Cardano/Api/Key/Internal/SomeAddressVerificationKey.hs Prevents invalid UTF-8 from throwing during Bech32 verification key deserialisation.
.changes/20260708_140000_cardano-api_pablo.lamela_nonthrowing_utf8_decode.yml Changelog fragment describing the bugfix behavior change.

Comment thread cardano-api/src/Cardano/Api/Key/Internal/SomeAddressVerificationKey.hs Outdated
Comment thread cardano-api/src/Cardano/Api/Serialise/DeserialiseAnyOf.hs Outdated

@Jimbo4350 Jimbo4350 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We shouldn't throw away the UnicodeException error. It needs to be rendered.

Comment thread cardano-api/src/Cardano/Api/Key/Internal/SomeAddressVerificationKey.hs Outdated
palas added 2 commits July 10, 2026 03:36
Instead of discarding the UnicodeException and fabricating
StringToDecodeContainsInvalidChars [], deserialiseAnyVerificationKeyBech32
now returns Bech32InvalidUtf8 carrying the rendered decoding error.
Restructure the bech32 branches of deserialiseInput and
deserialiseInputAnyOf as suggested in review.
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.

4 participants