[webkit-reviews] review denied: [Bug 36734] Implement symbolic CSS3 list-style-types : [Attachment 62485] Patch with test case

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 36734: Implement symbolic CSS3 list-style-types
https://bugs.webkit.org/show_bug.cgi?id=36734

Attachment 62485: Patch with test case
https://bugs.webkit.org/attachment.cgi?id=62485&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>      const int lettersSize = sizeof(number) * 8 + 1; // Binary is the worst
case; requires one character per bit plus a minus sign.

The new symbols sequence is now the worst case, much worse than binary. If you
don't change something there will be buffer overflow.

> Index: LayoutTests/fast/lists/w3-css3-list-styles-symbolic-expected.txt
> ===================================================================
> --- LayoutTests/fast/lists/w3-css3-list-styles-symbolic-expected.txt 
(revision 0)
> +++ LayoutTests/fast/lists/w3-css3-list-styles-symbolic-expected.txt 
(revision 0)
> @@ -0,0 +1,13 @@
> +CSS3 Symbolic list-style-type
> +
> +asterisks
> +
> +PASS
> +PASS
> +PASS
> +footnotes
> +
> +PASS
> +PASS
> +PASS
> +

Tests that just write out "PASS PASS PASS" are suboptimal. Can you change this
test so the output makes it clearer what is going on?

Do we need some kind of limit? I'm worried that you can generate millions of
asterisks and cause problems that way. Some limit would prevent that
possibility.

review- because I think this has a buffer overflow in it