[webkit-reviews] review denied: [Bug 53275] Fix ContentType parameter parsing error : [Attachment 88397] New patch based on Alexey's comments

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 6 12:27:01 PDT 2011


Eric Carlson <eric.carlson at apple.com> has denied Nancy Piedra
<nancy.piedra at nokia.com>'s request for review:
Bug 53275: Fix ContentType parameter parsing error
https://bugs.webkit.org/show_bug.cgi?id=53275

Attachment 88397: New patch based on Alexey's comments
https://bugs.webkit.org/attachment.cgi?id=88397&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88397&action=review

> Source/WebCore/ChangeLog:5
> +	   Parse quotes from content type codecs parameter

This comment is incorrect, the change is for any parameter not just "codecs".

> Source/WebCore/platform/ContentType.cpp:48
> -	       start = strippedType.find('=', start + 6);
> +	       start = strippedType.find('=', start + 1);

This isn't right, it starts the search at the beginning of the parameter name
instead of at the end.

> Source/WebCore/platform/ContentType.cpp:52
> +		   size_t quote = notFound;
> +		   size_t end = notFound;
> +		   if ((quote = strippedType.find('\"', start + 1)) != notFound
&& (end = strippedType.find('\"', start + 2)) != notFound) // find dobule quote


The initializations to notFound are not helpful because you always set quote
and end to strippedType.find().

I think it would be easier to read if you made the assignments outside of the
test, but that may just be me.


More information about the webkit-reviews mailing list