[webkit-reviews] review denied: [Bug 47382] Blob / BlobBuilder can be put into bad state with wild integers and strings : [Attachment 70174] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 7 17:17:38 PDT 2010


Jian Li <jianli at chromium.org> has denied Chris Evans <cevans at google.com>'s
request for review:
Bug 47382: Blob / BlobBuilder can be put into bad state with wild integers and
strings
https://bugs.webkit.org/show_bug.cgi?id=47382

Attachment 70174: Patch
https://bugs.webkit.org/attachment.cgi?id=70174&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
Thanks for finding and fixing the problem.

> WebCore/fileapi/BlobBuilder.cpp:69
> +    CString result = CString::newUninitialized(newLen, q);

Could you please also provide a test case for BlobBuilder.append?

> WebCore/ChangeLog:7
> +	   Fix integer errors in Blob and BlobBuilder.

Better to mention the problem in more details, like "Fix integer overflow
errors in Blob.slice and BlobBuilder.append".
Please update the bug title and ChangeLog to reflect this.

> LayoutTests/fast/files/blob-slice-oflow.html:11
> +var bb =  new BlobBuilder();

Better to name it as builder.

> LayoutTests/fast/files/blob-slice-oflow.html:12
> +var s= '';

Please add a space between 's' and '='.
Also, better to rename s as text.

> LayoutTests/fast/files/blob-slice-oflow.html:16
> +b = bb.getBlob();

Better to use the meaningful word to name the variable, like "blob".

> LayoutTests/fast/files/blob-slice-oflow.html:17
> +slice = b.slice(1999, 9223372036854775000);

Better to name it as slicedBlob.

> LayoutTests/fast/files/blob-slice-oflow.html:19
> +    'Blob slice length: ' + slice.size));

No need to split into 2 lines.

> LayoutTests/fast/files/blob-slice-oflow.html:22
> +  alert('FAIL');

Please log the failure instead of popping an alert dialog since otherwise it
will cause the test to hang on error.

> LayoutTests/ChangeLog:9
> +	   * fast/files/blob-slice-oflow.html: Added.

Would be better not to abbreviate the word. Please rename to
blob-slice-overflow.html.


More information about the webkit-reviews mailing list