Skip to content

fix(parse): match color function names and keywords case-insensitively - #275

Open
maximilliangrand wants to merge 1 commit into
Evercoder:mainfrom
maximilliangrand:fix/case-insensitive-parsing
Open

fix(parse): match color function names and keywords case-insensitively#275
maximilliangrand wants to merge 1 commit into
Evercoder:mainfrom
maximilliangrand:fix/case-insensitive-parsing

Conversation

@maximilliangrand

Copy link
Copy Markdown

CSS function names and keywords are ASCII case-insensitive, but culori accepts them only in lowercase:

parse('RGB(255 0 0)'); // → undefined (expected { mode:'rgb', r:1, g:0, b:0 })
parse('TRANSPARENT');  // → undefined (expected transparent black)

HSL(...), OKLCH(...), COLOR(display-p3 ...), legacy RGBA(…)/HSLA(…), and uppercase hue units (120DEG) are affected too — even though RED, RoyalBlue, and #FFF already parse.

Cause: the tokenizer preserves identifier case while every consumer compares lowercase only — the per-space function-name checks, the color() profile lookup, the hue-unit matching, the legacy comma-syntax rgb()/hsl() regexes, and the transparent keyword.

Fix: lowercase identifiers as the tokenizer consumes them (they are all case-insensitive keywords in the color grammar), add the i flag to the two legacy regexes (lowercasing the captured hue unit), and match transparent case-insensitively. Since lowercasing now feeds CONSTRUCTOR into the color() profile lookup, I guarded that lookup with an own-property check so identifiers inherited from Object.prototype (constructor, __proto__) return undefined instead of throwing — a latent crash that already hit their lowercase forms.

Added tests for the above; full suite is otherwise unchanged.

CSS function names and keywords are ASCII case-insensitive, but culori only
accepted them in lowercase: `RGB(255 0 0)`, `HSL(...)`, `OKLCH(...)`,
`COLOR(display-p3 ...)`, uppercase hue units (`120DEG`), and `TRANSPARENT`
all returned undefined, while `rgb(...)`/`red`/`RED`/`#FFF` worked.

- Normalize identifiers (function names, keywords, color-space names, hue
  units) to lowercase as they are consumed by the tokenizer.
- Add the `i` flag to the legacy comma-syntax rgb()/hsl() regexes and lowercase
  the captured hue unit before mapping it.
- Match `transparent` case-insensitively.
- Guard the color() profile lookup with an own-property check so identifiers
  inherited from Object.prototype (`constructor`, `__proto__`, …) resolve to
  no profile and return undefined instead of throwing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@danburzo danburzo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for this PR. I’ve made a few comments, but otherwise looks good!

Comment thread src/hsl/parseHslLegacy.js
if (match[3] !== undefined) {
res.h = +match[3];
} else if (match[1] !== undefined && match[2] !== undefined) {
res.h = hueToDeg(match[1], match[2]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just as a stylistic preference, could you move toLowerCase() to the hueToDeg() function directly?

switch (unit?.toLowerCase()) {  }

@@ -1,5 +1,5 @@
const parseTransparent = c =>
c === 'transparent'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be useful to guard for undefined colors with c?.toLowerCase().

Comment thread src/parse.js
while (_i < chars.length && IdentCodePoint.test(chars[_i])) {
v += chars[_i++];
}
return v;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As noted in #276, I think instead of all the changes here we just make the input lowercase in the tokenize() function.

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.

2 participants