[Webkit-unassigned] [Bug 116096] DOMFileSystemBase: merge BlackBerry implementation

[bugzilla-daemon at webkit.org]

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


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #201699|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #5 from Benjamin Poulain <benjamin at webkit.org>  2013-05-14 23:48:38 PST ---
(From update of attachment 201699)
View in context: https://bugs.webkit.org/attachment.cgi?id=201699&action=review

It is awesome you are merging the two code. The KURL thingy is pretty bad though.

>>> Source/WebCore/Modules/filesystem/DOMFileSystemBase.cpp:87
>>> +        KURL originURL(ParsedURLString, url.path());
>> 
>> Is it really OK to parse just the path as a URL and claim it is a “parsed URL string”? What are you trying to accomplish here? How will originURL.path() be different from url.path()?
> 
> The filesystem url is something like "filesystem:http://example.com/temporary/myfile.png", which is basically a wrapper. So url.path() is "http://example.com/temporary/myfile.png" and originalURL.path() is "/temporary/myfile.png"

Darin is absolutely right, this is completely wrong.

ParsedURLString should _ONLY_ be used if the input string was already parsed and canonicalized by KURL. Code like this is a great way to get security problems.

> Source/WebCore/Modules/filesystem/DOMFileSystemBase.cpp:100
> +        path = path.substring(1);
> +
> +        if (path.startsWith(temporaryPathPrefix)) {
> +            type = FileSystemTypeTemporary;
> +            path = path.substring(temporaryPathPrefixLength);
> +        } else if (path.startsWith(persistentPathPrefix)) {
> +            type = FileSystemTypePersistent;
> +            path = path.substring(persistentPathPrefixLength);
> +        } else
> +            return false;

This is very inefficient code, I don't know if you care about performance here?

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