[css-values] minimum nested pairs of parentheses in calc to 32

[TalbotG]

calc-errors.html test at line 68

At line 68 of the test above, there is a calc() function with 128 nested parentheses. Firefox is able to support 128 pairs of nested parentheses. Blink supports 32 nested pairs of parentheses and then fails when trying 33 nested pairs of parentheses:

trac-webkit-calc-errors.html with 33 nested pairs of parentheses

128 nested pairs of parentheses seems outrageously extreme but there seems nothing in the CSS3 spec and CSS4 spec that indicate ceiling limit for nested parentheses.

Proposal: CSS4 spec explicitly state a limit (ceiling) of support to 32 nested pairs of parentheses in calc() function.

Then the test calc-errors.html should be adjusted accordingly before submitted to wpt.

[TalbotG]

Same issue in this

calc-errors.html test at line 59

[emilio]

Why? And why only on calc() and not on other (all?) CSS blocks?

[TalbotG]

There ought to be a reasonable ceiling limit to everything. 32 seems to me very reasonable.
Right now, Blink supports 32 but not 33 nested parentheses; Firefox supports 128.

and not on other (all?) CSS blocks?

Which other CSS blocks? I'm not sure I understand.

[emilio]

I mean why only on calc() and not on, e.g., nested @media rules and such?

[ewilligers]

Another example is CSS Selectors 4 :is(:not(...))

When the spec has no limits, implementations are forced to make their own choices between

• having well known and ignored stack overflow crashes, e.g. on fuzzer-generated input. The smallest example that crashes can change when moving to a different compiler version or when adding an unrelated feature.
• not using their implementation language's support for recursion, complicating their code and increasing the development cost of future features.
• choosing their own limits and making an intentional choice to fail any WPTs that go further.

[Martin-Pitt]

Ideally the limit should be a minimum so that authors can have guarantee and as technical limits matter less in the future the ceiling can be lifted and use cases may occur we did not realise.

A spec should never really have a maximum limit in my opinion.

Is there precedence for min/max limits? What about limits on amount of CSS selectors or how many CSS stylesheets you can import or link to? Is a limit here a dangerous precedent or would asking implementations to guarantee some minimum be more reasonable?

[TalbotG]

Right now, Firefox fails
calc-errors.html test
only because it supports much more than 32 nested parentheses; it supports 128 nested parentheses.

If we can not reach an agreement on a ceiling limit value on calc nested parentheses, the very least minimum to do will be to adjust such test.

[emilio]

That test is a WebKit-internal test, I don't see why Firefox would need to pass it.

[tabatkins]

CSS has never introduced actual maximums for anything. We have a few places where we've defined minimums that must be supported (with the implication that going over is implementation-defined and allowed to fail, in a particular spec-defined fashion).

@fantasai and I have tried to introduce such limits for all the numeric values, but haven't succeeded yet. :(

I'm happy to similarly define a maximum required nesting limit here. Presumably a separate nesting limit for rules vs values, since they're generally parsed in different phases?

When the spec has no limits, implementations are forced to make their own choices between [snip]

Crashing is never a reasonable behavior. ^_^

[TalbotG]

That test is a WebKit-internal test, I don't see why Firefox would need to pass it.

I will be submitting that test - most likely modified a bit - to WPT repository soon (say, within 6 weeks). So the test has to be modified somehow so that Firefox is not unfairly penalized (and does not fail) just because it supports 128 nested calc parentheses.

[Loirooriol]

I'm happy to similarly define a maximum required nesting limit here

@tabatkins Did you mean minimum? Otherwise, why add this inconsistency if minimums are used everywhere else?

[fantasai]

I would agree with setting a minimum, but not a maximum, in the spec.

As for the test, it could be written with as many parentheses as needed to pass in all known implementations, and also include a comment to the effect that if your UA is failing because you support more, then ask the test to be adjusted. It might also be worth separating out that test into its own file.

[TalbotG]

As for the test, it could be written with as many parentheses as needed to pass in all known implementations

I will eventually adjust that sub-test so that only 32 nested parentheses are used.
I am leaving this issue opened until this is done.

[tabatkins]

Sorry, yes, that's what I meant. Must support up to X degree of nesting; can support more but are not required to; must fail in a particular standardized way when the limit is exceeded.

[TalbotG]

I will be submitting that test - most likely modified a bit - to WPT repository soon (say, within 6 weeks). (...)
I will eventually adjust that sub-test so that only 32 nested parentheses are used.

Following-up on the above.
I am no longer involved into submitting that test. @frivoal is:
webkit-css-values-tests

I think that 128 pairs of nested parentheses inside a calc() function in that test should be reduced to 32 pairs. Note that there was already a test from @FremyCompany that was testing 24 pairs of nested parentheses inside a calc() function:
calc-parenthesis-stack.html

[TalbotG]

It might also be worth separating out that test into its own file.

Then François Remy's calc-parenthesis-stack.html test could or would suffice.

[ExE-Boss]

I believe that CSS implementations should support a minimum of 32 nested parentheses.

[frivoal]

agenda+ to confirm #3462 (comment)

[AmeliaBR]

The current spec in CSS Values is

UAs must support math function expressions of at least 20 terms, where each NUMBER, DIMENSION, or PERCENTAGE is a term. If a math function contains more than the supported number of terms, it must be treated as if it were invalid. (emphasis added)

[FremyCompany]

Just reviewed the test, and we might also want to make clear whether the parentheses for calc() itself are included in the 32 requirement or not. If they are not, we should update the proposed test to have one more nesting level, I believe.

[tabatkins]

@AmeliaBR Right, but that doesn't restrict the number of parens you have to be able to handle; you could have a single "term" wrapped in a hundred paren pairs and not violate that support requirement. ^_^

Tho I suppose there's no reason to mandate at least 32 nesting levels and mandate less than 32 terms, since the only reason to add parens is to separate terms. We should probably bump the minimum terms requirement to 32 as well, then.

I think I also need to rewrite that requirement a bit, as it's implicitly (and vaguely) referring to tokens; as stated, an expression containing a single 1 and a thousand attr(foo px) terms would be required to be supported.

[css-meeting-bot]

The CSS Working Group just discussed minimum nested pairs of parentheses in calc to 32, and agreed to the following:

• RESOLVED: Set minimum number of parenthesis to 32
• RESOLVED: Raise the number of expressions that must be handled to 32

The full IRC log of that discussion

<dael> Topic: minimum nested pairs of parentheses in calc to 32
<dael> github: https://github.com//issues/3462#issuecomment-478408798
<dael> TabAtkins: Get a resolution that we should have limit on req. number nested parans that impl must support
<dael> TabAtkins: calc imposes min on terms but there's no spot where we allow you to fail with nested
<dael> TabAtkins: Prop: Put in a mandatory min for paren depth. Past that you're allowed to fail. Number is set to 32
<dael> fantasai: And this is a min, not a max.
<dael> florian: It's setting a min and saying when you fail how you do so?
<tantek> regrets+
<dael> TabAtkins: Yes, we have that term in other parts of calc so reuse
<dael> AmeliaBR: This is specfically about parsing nesting so this is literal parens not conseptual order of operations? Chrome when serializes adds a lot of parens so from authoring might be confusing. One paren with multi operations is it literal parens?
<dael> TabAtkins: It's a parsing problem so literal parens. Further order of operations can only impose one additional level so don't have to worry about that nesting indefinetly.
<dael> florian: If chrome inserts unnec parens it's probably wrong.
<dael> TabAtkins: Yep
<dael> AmeliaBR: True but I think 32 number comes out of chrome so not sure if anybody has tested how the 32 is counted in chrome and maybe has an effect?
<dael> florian: Are we saying 32 included or excluded?
<dael> TabAtkins: I do not care. I'll put one down.
<dael> fremy: Similar question because it's a difference of one. Need to be clear if it's inside the code.
<dael> florian: As long as clear doesn't matter
<dael> fremy: Agree
<dael> astearns: Serialization bug will be an extra problem if people using serialized values to set other prop. Importance of fixing prob. go up
<dael> florian: But it's a min not a max.
<dael> astearns: We have a limit on terms which is 20, why choose different number?
<bkardell_> can someone explain why that is not a good assumption?
<bkardell_> (that you should be able to round-trip parse/serialize)
<fantasai> bkardell_, because browsers have bugs :)
<bkardell_> are these bugs not fixable?
<fantasai> bkardell_, in an ideal state, it should otherwise be a good assumption
<dael> TabAtkins: 20 terms came because it seemed good. 32 is smallest limit across the browsers; Blink has 32. It would prob. be good to have similar numbers so I suggest raising term to 32 to make them symmertic
<fantasai> bkardell_, yes
<dael> AmeliaBR: I don't think it's conceptually possible to support 32 nesting and 20 terms. Doesn't bracket count as a term?
<bkardell_> fantasai: yes they are fixable, or yes they are not fixable? :)
<fantasai> bkardell_, fixable of course :)
<dael> TabAtkins: No. It doesn't count functions or parens so I need to update. When it's updated paren groups should count
<dael> astearns: bkardell_ asked on IRC about what's not a good assumption. We should not assume people writing serialization code and people writing paren code are talking to each other. Assuming if i's all chrome it'll work is bad.
<dael> bkardell_: SO we want to spec so when done it's safe assumption?
<dael> florian: We want chrome to fix bugs b/c having more parens in output then in input is not a thing that should happen. By shortest serialization it shouldn't do that
<dael> bkardell_: AS we spec it there should be a test that says that is a safe assumption
<dael> astearns: Should be a bug for a 32 paren that's parsed, you serialize, reparse, and then it works.
<dael> astearns: I believe prop is?
<dael> TabAtkins: Make the term limit consistent with nested parens limit
<dael> astearns: 2 resolutions. Set a min parens that is set at 32. Then raise min to 32
<AmeliaBR> s/raise min/raise min terms/
<dael> astearns: Obj to set minimum number of parenthesis to 32
<dael> RESOLVED: Set minimum number of parenthesis to 32
<dael> astearns: Obj to raise the number of expressions that must be handled to 32
<dael> RESOLVED: Raise the number of expressions that must be handled to 32
<dael> astearns: Lots of tests for all of this
<dael> florian: Writing a test is what prompted it
<dael> TabAtkins: For values stuff I'm enjoying writing tests so I'll make sure inputs go in with tests

[TalbotG]

@frivoal
Florian, You now need to remove lines 76 and 77 from calc-errors.html before submitting that test. You will also need to remove lines 72 and 73 from calc-errors-ref.html .

Someone (it could be you, Florian) will need to approve pull request 16170 now that this issue has been settled. Pull request 16170 involves a specific, dedicated test on a calc() with 32 pairs of parentheses.

[frivoal]

Florian, You now need to remove lines 76 and 77 from calc-errors.html before submitting that test. You will also need to remove lines 72 and 73 from calc-errors-ref.html.

done.