Skip to content

fix: best-efforts handling of poisoned (unencoded) Readme descriptions during deserialization#2593

Merged
cmgrote merged 2 commits into
mainfrom
fix/decode-content-illegal-percent
Jul 13, 2026
Merged

fix: best-efforts handling of poisoned (unencoded) Readme descriptions during deserialization#2593
cmgrote merged 2 commits into
mainfrom
fix/decode-content-illegal-percent

Conversation

@kabhishek1001

@kabhishek1001 kabhishek1001 commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

What

URLDecoder.decode() throws IllegalArgumentException when it encounters a % not followed by exactly two valid hex digits. AssetDeserializer calls StringUtils.decodeContent() on the description of every Readme asset during index search response deserialization. If any Readme in the result batch has plain text like "50% faster" in its description, the exception propagates through Jackson and crashes deserialization of the entire batch — not just the offending asset.

Fix

Catch IllegalArgumentException in decodeContent() and return the original string. Legitimately URL-encoded descriptions (the normal case) continue to decode correctly. Plain-text descriptions that happen to contain % are returned as-is, which is the right behavior, as they were never URL-encoded in the first place.

Testing

Added decodeContentWithLiteralPercent() to StringUtilsTest covering the two patterns seen in production (%; and % u) plus the null case. Existing encodeDecodeContent() test confirms the happy path is unaffected.

URLDecoder.decode() throws IllegalArgumentException when a string
contains % not followed by two valid hex digits. Readme descriptions
stored as plain text (e.g. "50% faster") hit this when the
AssetDeserializer calls decodeContent() during an index search response.

Fall back to returning the original string when decoding fails, so
a malformed or plain-text description doesn't crash deserialization
of the entire batch.
@kabhishek1001
kabhishek1001 requested a review from cmgrote as a code owner June 29, 2026 06:01
@cmgrote

cmgrote commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Solid defensive fix — worth being explicit about what it does and doesn't address.

This catches the symptom safely, but it's worth noting for the record that the underlying issue is upstream: a readme description containing a bare % can only reach us if it was persisted without going through encodeContent in the first place. Both the UI and AssetSerializer encode on write (URLEncoder.encode turns a literal % into %25, which round-trips back cleanly and never throws), so this data came in via a non-standard path — a direct API call or integration that set description to raw text/HTML. The most robust fix is to find and fix that writer so it encodes properly.

That said, this change is still worth shipping on its own merits:

  • No risk to correctly-encoded data. It never throws, so the new catch is never reached — behavior is byte-for-byte identical for the normal case.
  • Contains the blast radius. Today one malformed readme crashes deserialization of the entire search batch via Jackson, not just the offending asset. This limits the damage to that one field.

Two things to keep on the radar so we don't consider this closed:

  1. It's imperfect even as a symptom fix. The try/catch only rescues strings that throw (bare % + non-hex, e.g. % u, %;). Un-encoded text that happens to look like valid encoding — literal %20, %3C, etc. — won't throw, so it gets silently mis-decoded with no warning. We can't distinguish "encoded" from "coincidentally-looks-encoded" at this layer.
  2. Existing bad data. Even once the writer is fixed going forward, poisoned readmes may already exist in tenants. Ideally we chase the writer and backfill/re-encode the existing rows.

TL;DR: 👍 to stop the bleeding and contain blast radius — but let's open a follow-up to track down the writer producing un-encoded descriptions.

@cmgrote
cmgrote enabled auto-merge July 13, 2026 09:31
@cmgrote cmgrote changed the title fix: handle literal % in Readme descriptions during deserialization fix: handle poisoned (unencoded) Readme descriptions during deserialization Jul 13, 2026
@cmgrote cmgrote changed the title fix: handle poisoned (unencoded) Readme descriptions during deserialization fix: best-efforts handling of poisoned (unencoded) Readme descriptions during deserialization Jul 13, 2026
@cmgrote
cmgrote merged commit 8e04424 into main Jul 13, 2026
5 checks passed
@cmgrote cmgrote added the bug Something isn't working label Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants