Skip to content

fix(core): reject malformed color input instead of producing NaN - #205

Merged
pbroom merged 4 commits into
mainfrom
fix/core-input-validation
Oct 10, 2026
Merged

pbroom merged 4 commits into
mainfrom
fix/core-input-validation

Conversation

@pbroom

@pbroom pbroom commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

parse('#gggggg') returned { l: NaN, c: NaN, h: NaN } because hexToRgb
never validated its digits, toHex then emitted "#NaNNaNNaN", inputs such
as rgb(1.2.3 0 0) were accepted, and toCss silently fell back to hex for
unknown formats.

  • hexToRgb validates length (3/4/6/8) and digits and throws otherwise.
  • parse moves to conversion/parse.ts and validates every component token
    against the CSS grammar, so malformed numbers throw and
    components that overflow to Infinity (1e999) throw too.
  • New tryParse(input): Color | null, exported from the root. A nullable
    return matches the existing parseColorStringInputValue helper in
    @color-kit/driver and keeps the hot path allocation-free; a result
    object would add API surface without carrying more than the error
    message, which parse already provides.
  • toCss takes a CssColorFormat string-literal union and throws a
    TypeError for unknown formats at runtime. @color-kit/react's
    requestedCss/displayedCss callbacks now take CssColorFormat.
  • toHex, toCss and rgbToHex throw a RangeError for non-finite channels
    instead of emitting NaN digits.
  • Tests include fast-check properties: arbitrary garbage never parses to
    a non-finite Color, and tryParse never throws.

Bundle size: parse + toHex + contrastRatio 2.32 kB -> 2.64 kB (limit
2.7 kB, unchanged).

Part of a stacked set of PRs (#204 → #221) from the color-kit assessment. Validated per branch with pnpm build and pnpm test; lint, format, typecheck, size, artifact, preprod and agents checks pass at the top of the stack.

🤖 Generated with Claude Code

pbroom commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Adds input validation to color parsing and serialization.

The PR should not merge until parsing rejects colors that become non-finite during conversion.

Fix All in CursorFindings

  1. P1 Conversion can return infinite chroma ▶

Summary

The PR validates hex digits and CSS color components, adds nullable tryParse(), rejects non-finite serializer inputs and unknown CSS output formats, and narrows React formatting callback types. One conversion path can still return a non-finite color from finite input tokens.

Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Input["OKLab text input"] --> Token["Validate finite component tokens"]
  Token --> Convert["Convert OKLab to OKLCH"]
  Convert --> Result["parse returns Color"]
  Result --> TryParse["tryParse returns Color"]
  Convert -. "Large finite components can overflow here" .-> Result
Loading

Reviews (1) · Last reviewed commit: "fix(core): reject malformed color input ..."

Comment thread packages/core/src/conversion/parse.ts
@vercel

vercel Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
color-kit Ready Ready Preview Oct 10, 2026 6:58pm UTC

pbroom commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner Author

Merge activity

  • Oct 10, 6:47 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Oct 10, 6:57 PM UTC: Graphite rebased this pull request as part of a merge.
  • Oct 10, 6:58 PM UTC: @pbroom merged this pull request with Graphite.

@pbroom
pbroom changed the base branch from chore/assessment-dev-deps to graphite-base/205 October 10, 2026 18:54
@pbroom
pbroom changed the base branch from graphite-base/205 to main October 10, 2026 18:55
pbroom and others added 4 commits October 10, 2026 18:56
parse('#gggggg') returned { l: NaN, c: NaN, h: NaN } because hexToRgb
never validated its digits, toHex then emitted "#NaNNaNNaN", inputs such
as rgb(1.2.3 0 0) were accepted, and toCss silently fell back to hex for
unknown formats.

- hexToRgb validates length (3/4/6/8) and digits and throws otherwise.
- parse moves to conversion/parse.ts and validates every component token
  against the CSS <number> grammar, so malformed numbers throw and
  components that overflow to Infinity (1e999) throw too.
- New tryParse(input): Color | null, exported from the root. A nullable
  return matches the existing parseColorStringInputValue helper in
  @color-kit/driver and keeps the hot path allocation-free; a result
  object would add API surface without carrying more than the error
  message, which parse already provides.
- toCss takes a CssColorFormat string-literal union and throws a
  TypeError for unknown formats at runtime. @color-kit/react's
  requestedCss/displayedCss callbacks now take CssColorFormat.
- toHex, toCss and rgbToHex throw a RangeError for non-finite channels
  instead of emitting NaN digits.
- Tests include fast-check properties: arbitrary garbage never parses to
  a non-finite Color, and tryParse never throws.

Bundle size: parse + toHex + contrastRatio 2.32 kB -> 2.64 kB (limit
2.7 kB, unchanged).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ation

The non-finite result check adds about 30 B gzipped (2.73 kB measured).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…e time

- Match function arguments across lines, so newlines, tabs, CR and FF
  between components parse as in CSS (the old parser accepted them).
- Clamp per CSS Color 4 at parsed-value time: negative HSL saturation
  to 0% (hsl(0 -100% 50%) is gray, not cyan), OKLab/OKLCH lightness to
  [0, 1], and negative OKLCH chroma to 0.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@pbroom
pbroom force-pushed the fix/core-input-validation branch from bf2d047 to d5b1919 Compare October 10, 2026 18:56
@pbroom
pbroom merged commit f65d855 into main Oct 10, 2026
6 of 7 checks passed
@pbroom
pbroom deleted the fix/core-input-validation branch October 10, 2026 18:58
pbroom added a commit that referenced this pull request Oct 10, 2026
CSS Color 4 legacy rgb() needs all-number or all-percentage channels and
legacy hsl() needs percentage saturation and lightness; #205 enforced
this, and the CSS Color 4 parser accepted rgb(100%, 0, 0) and
hsl(0, 100, 50). The modern space syntax still allows mixing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

This branch was successfully deployed

1 active deployment
Preview — d5b19196 Deployed Oct 10, 2026 by vercel[bot]
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