Skip to content

chore: sweep every decode path for strict fallback and refused-conversion discipline #580

Description

Summary

While implementing the Uri formatter for WallstopProto this week, two different silent-decode failure modes were hit, both inside one method boundary. Both would recur anywhere else in production code that decodes bytes into values. This asks for a scan and implementation pass over Runtime/ (and a grep-only pass over Editor/) to make strict decoding deliberate everywhere.

The two classes found

1. Replacement-fallback decoding corrupts instead of refusing.

Encoding.UTF8.GetString(invalidBytes) replaces invalid sequences with U+FFFD rather than reporting them. A decoder wired that way turns hostile or truncated input into a different, valid-looking value — for our case, a wrong Uri — with no error surface:

// Today's fix: the only decoder that refuses
private static readonly UTF8Encoding StrictUtf8 = new UTF8Encoding(false, true);

Scope of suspicion (rg -n "Encoding.UTF8.GetString|GetString\(" Runtime/): JSON converters (WGuidConverter, Hash128Converter), binary helpers reading UTF-8 prefixes, compression paths, anything taking network/file bytes. Sites where the bytes provably came from our own writer may legitimately keep the forgiving decoder, but that needs to be a per-site decision, not an accident of which API was nearest.

2. Parser laxness on text -> value conversion.

The malformed-corpus probe also discovered new Uri("http://example.com/%AAA") parses happily while %AAA in authority position throws — "looks like it should fail" is not acceptance logic. Anywhere we convert decoded strings to typed values (Uri, Type.GetType-style lookups, enum parsing, version strings) should answer refuse-vs-accept from a stated rule, and refuses must be observable (TryX pattern returning false) rather than defaulted.

What "done" means

  • An inventory of every byte/character decode site under Runtime/, each marked strict / forgiving / ours-only, with the owning rationale.
  • Forgive-mode sites carry a comment naming why corruption is impossible there.
  • Untrusted-input decode sites use throwing decoders (throwOnInvalidBytes) or pre-validation, converting failures into the package's standard graceful-refusal (no throws crossing public APIs).
  • Malformed-payload tests exist for each newly-strict path (the BclDifferentialTests malformed section is the pattern).
  • Editor/ gets a grep-level audit recorded here as evidence, not full parity; editor-only decoders feeding runtime saves are the exception worth checking.

Non-goals

No new linter/diagnostic unless the inventory shows a recurring shape a human keeps re-mistaking — measure first.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions