Skip to content

fix(datasource-customizer): reject malformed binary filter values instead of crashing - #1835

Open
Tonours wants to merge 3 commits into
mainfrom
fix/binary-datauri-guard
Open

fix(datasource-customizer): reject malformed binary filter values instead of crashing#1835
Tonours wants to merge 3 commits into
mainfrom
fix/binary-datauri-guard

Conversation

@Tonours

@Tonours Tonours commented Aug 18, 2026

Copy link
Copy Markdown
Member

What

A Binary column filtered or written with a malformed value now raises ValidationError instead of crashing or converting the wrong bytes.

Input Before After
'Anthony' on a datauri column TypeError, untyped → 500 ValidationError
'Anthony' on a hex column (every Binary PK/FK) empty buffer, wrong results, no error ValidationError
'data:text/plain,hello' decoded to garbage (85e965) ValidationError
42 TypeError, untyped → 500 ValidationError

Why

convertScalar assumed its input was always a well-formed datauri or hex string. It isn't: the value comes from a filter or a record patch, so anything can arrive.

Buffer.from('Anthony'.split(',')[1], 'base64') throws a raw TypeError — no HTTP status, so it surfaces as a generic 500 and the caller never learns what to send instead.

The hex branch was worse than the crash: Buffer.from never throws on bad hex, it stops at the first invalid character. 'Anthony' yields an empty buffer, so a filter on a Binary primary key silently queried for an empty blob and returned wrong results with no error at all. shouldUseHex defaults to true for every primary and foreign key, so that was the common path.

How

  • non-string value rejected up front instead of being cast
  • datauri branch delegates to the toolkit's parseDataUri, dropping a hand-rolled split that missed its hardening
  • hex branch gets /^([0-9a-f]{2})*$/i, mirroring the Match validation this decorator already advertises on hex fields — odd lengths included, since Buffer.from('303','hex') silently drops a digit

5 tests, each asserting the message and that the underlying collection was never called.

Heads-up: writes are stricter too

create/update go through the same conversion. Three datauri forms that previously reached the database now raise: a raw comma in a name= media type, data:base64,… with no semicolon, and uppercase BASE64 (parseDataUri is case-sensitive, RFC 2397 is not).

These rules come from parseDataUri, which already applies them to the S3 plugin's write path. If the uppercase form matters, fix it there for every caller rather than special-casing this decorator.

Test

yarn workspace @forestadmin/datasource-customizer test -- test/decorators/binary/collection.test.ts

Also verified against a live agent: the same filter went from an opaque 500 to a 400 naming the expected format.

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

@qltysh

qltysh Bot commented Aug 18, 2026

Copy link
Copy Markdown

1 new issue

Tool Category Rule Count
qlty Structure Function with many returns (count = 4): convertScalar 1

@qltysh

qltysh Bot commented Aug 18, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
...ages/datasource-customizer/src/decorators/binary/collection.ts100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

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.

1 participant