[webkit-reviews] review granted: [Bug 47309] Fix FileSystem path validation order to normalize '..' and '.' before restriction checks : [Attachment 70168] Patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Kinuko Yasuda
<kinuko at chromium.org>'s request for review:
Bug 47309: Fix FileSystem path validation order to normalize '..' and '.'
before restriction checks
https://bugs.webkit.org/show_bug.cgi?id=47309

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

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70168&action=review

> LayoutTests/fast/filesystem/resources/op-get-entry.js:60
> +	       function(helper) { helper.getFile('/a',
'/a/../../b/./c/../../../../../e.txt', {create:true}); },

Consider adding a test with "..." to check that it is invalid (unless you
already have one).

> WebCore/fileapi/DOMFileSystemBase.cpp:145
> +    if (DOMFilePath::isAbsolute(path))

How about: 
if (!DOMFilePath::isAbsolute(path))
   path = DOMFilePath::append(base->fullPath(), path);
String absolutePath = DOMFilePath::removeExtraParentReferences(path);

In fact make this all into a function b/c there is so much common code in these
two places.

Feel free to change names but this is what I'm thinking:

bool DOMFileSystemBase::domPathToPlatformPath(String domPath, String&
platformPath)
{
    if (!DOMFilePath::isAbsolute(path))
	domPath = DOMFilePath::append(base->fullPath(), domPath);
    String absolutePath = DOMFilePath::removeExtraParentReferences(path);

    if (!DOMFilePath::isValidPath(absolutePath))
	return false;
    platformPath = m_asyncFileSystem->virtualToPlatformPath(absolutePath);
    return true;
}

String platformPath;
if (!domPathToPlatformPath(path, platformPath))
    return false;