[webkit-reviews] review denied: [Bug 30827] Off-by-one hard-to-trigger memory corruption in CSSParser (seen only with GCC 4.4) : [Attachment 42244] patch and layout test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 30 16:40:15 PDT 2009


Darin Adler <darin at apple.com> has denied  review:
Bug 30827: Off-by-one hard-to-trigger memory corruption in CSSParser (seen only
with GCC 4.4)
https://bugs.webkit.org/show_bug.cgi?id=30827

Attachment 42244: patch and layout test
https://bugs.webkit.org/attachment.cgi?id=42244&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Next time, please set review? on a patch to indicate you'd like review. Unless
you intentionally left that out here.

> +<style>tenbytes {</style>
> +
> +This test tickles a subtle off-by-one bug in how WebKit's CSS lexer
> +handles end of buffer conditions.<p>
> +
> +The contents of the style tag satisfy (length mod 8 = 2) and contain an
> +unclosed curly brace.  We pass if we don't crash.<p>
> +
> +PASS

The use of <p> here is incorrect. It's a container, not a separator. <p> goes
at the start of a paragraph and then </p> at the end of it.

This test needs to have some code to make it dump text-only results so we have
a platform-independent test. Like this:

    <script>
    if (window.layoutTestController)
	layoutTestController.dumpAsText();
    </script>

The patch needs to include expected results for the test, generated by running
the run-webkit-tests script.

Patch otherwise looks very good. Great fix!

review- because the test needs results


More information about the webkit-reviews mailing list