d3 / d3-color

Color spaces! RGB, HSL, Cubehelix, CIELAB, and more.
https://d3js.org/d3-color
ISC License
398 stars 91 forks source link

avoid backtracking when parsing #89

Closed mbostock closed 2 years ago

mbostock commented 3 years ago

This seems to address the issue, but is slightly too lax, and I would like to avoid the tiny amount of code duplication. And I haven’t tested performance in the non-degenerate case.

mbostock commented 3 years ago

It also looks like modern browsers are more lax, for example, they don’t distinguish between rgb and rgba or hsl and hsla. I’ve done that, but I think we’re still a little too lax in the sense of allowing percents and non-percents to be mixed.

mbostock commented 3 years ago

I’m worried that if our parser is too lax, someone will parse a color with d3-color and assume that it’s valid, and then try to use the same string with the browser natively where it will fail. I think it’s okay if the inverse is not true, i.e. if a browser fully supports CSS Color Level 4 but d3-color only supports Level 3 (plus perhaps some subset of Level 4).

Fil commented 2 years ago

For reference, there is a parser here https://github.com/deanm/css-color-parser-js/blob/master/csscolorparser.js (but it doesn't make exactly the same choices).

mbostock commented 2 years ago

I probably need to bite the proverbial bullet and implement a proper tokenizer and parser here.

kulak-at commented 2 years ago

Is there any update on that? It would be great to have this merged as it's affecting a lot of packages: https://security.snyk.io/vuln/SNYK-JS-D3COLOR-1076592

mbostock commented 2 years ago

Superseded by #99.