[Webkit-unassigned] [Bug 41510] Add ignoreWhitespace option at base64Decode()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41510





--- Comment #4 from Darin Adler <darin at apple.com>  2010-07-02 12:05:52 PST ---
(From update of attachment 60348)
Looks pretty good.

> -    if (!base64Decode(in, out)) {
> +    if (!base64Decode(in, out, false)) {

When a boolean argument is one that we'll pass constants to, we normally prefer an enum. There's no way to see what that "false" means at the call site. So we'd do this:

    enum WhitespacePolicy { FailIfWhitespaceIsEncountered, SkipWhitespace };

Then make the argument type be WhitespacePolicy.

> -    // Note: Keep this in sync with the "out_len" computation below.
> +    // Note: Keep this in sync with the "outLen" computation below.

As long as you have to rename this, I think I would have avoided the abbreviation and named it outLength.

> +    // Allowed because String::ascii() converts invalid charcters to '?',

Typo: "charcters".

> +    // which isn't contained in the base64 characterset.

Typo: "characterset".

I suggest this comment:

    // The ascii() function turns any non-ASCII characters into "?" characters,
    // which yields correct behavior because those are illegal in base 64.

>      // 4-byte to 3-byte conversion
> -    unsigned outLen = len - ((len + 3) / 4);
> +    unsigned outLen = len - whitespaceCount - ((len + 3) / 4);

I believe this is wrong. It should be "len - whitespaceCount - ((len - whitespaceCount + 3) / 4)". In cases with a lot of whitespace, the code you wrote would result in a buffer overrun!

Maybe you could find a way to write it that would make that clear. Perhaps add a buffer-overrun-related assertion, and make sure you have a test case with a ton of whitespace in it.

review- because of the buffer overrun issue

As far as testing the use of a data URL for a user stylesheet, I don't think we have a good way to regression-test that at the moment -- we'd have to add something

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.