[fsp] Fix incorrect handling of file system URLs when containing a %.

webkit/common/fileapi/file_system_util.cc

Index: webkit/common/fileapi/file_system_util.cc
diff --git a/webkit/common/fileapi/file_system_util.cc b/webkit/common/fileapi/file_system_util.cc
index a19136a1e9e20acf7add98ae88eb4f896a112350..ded3457ccf3efb2626c51f6491701a7adefefca5 100644
--- a/webkit/common/fileapi/file_system_util.cc
+++ b/webkit/common/fileapi/file_system_util.cc
@@ -454,12 +454,12 @@ std::string GetIsolatedFileSystemRootURIString(
                                           kFileSystemTypeIsolated).spec();
   if (base::FilePath::FromUTF8Unsafe(filesystem_id).ReferencesParent())
     return std::string();
-  root.append(filesystem_id);
+  root.append(net::EscapePath(filesystem_id));

[kinaba, 2014/06/06 05:40:11]

Maybe EscapeQueryParamValue is more appropriate?

According to the comment, net::EscapePath doesn't escape '/'.

[mtomasz, 2014/06/06 05:45:37]

Mount point names must not contain "/", per:
https://code.google.com/p/chromium/codesearch#chromium/src/webkit/browser/fil...

However, there is no restriction on the problematic %. So I believe, we don't
need to escape "/" here, since it is not allowed at all.

Note, that file system provider escapes "/" when generating the mount point
name:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

However, it seems that escaping "%" and "." in mount_path_util.cc is completely
redundant, and we should just escape "/" there.

Yet another solution is to remove the restriction on "/" in the mount point
name, and use EscapeQueryParamValue here, instead of EscapePath.

WDYT?

[kinaba, 2014/06/06 06:10:47]

Ah, thanks, I hope I've got your point.
So, the assumption is that the strings touched in this patch (filesystem_id,
mount_name)
are already avoiding '/' in some way (actually by escaping by '%2F'),
and what you want to handle here is the '%' characters here, right?


On 2014/06/06 05:45:37, mtomasz wrote:
If you want to treat ".." as a valid id in fileSystemProvider,
you still may want to escape dots. Otherwise the ReferencesParent() in
the line above will reject it.

[mtomasz, 2014/06/06 06:14:43]

You're right. In mount_path_util.cc, "." need to be escaped, as well as "%",
since otherwise we would have a collision:
%25-Wine and .-Wine both would resolve to %25-Wine.

   root.append("/");
   if (!optional_root_name.empty()) {
     if (base::FilePath::FromUTF8Unsafe(optional_root_name).ReferencesParent())
       return std::string();
-    root.append(optional_root_name);
+    root.append(net::EscapePath(optional_root_name));
     root.append("/");
   }
   return root;
@@ -472,7 +472,7 @@ std::string GetExternalFileSystemRootURIString(
                                           kFileSystemTypeExternal).spec();
   if (base::FilePath::FromUTF8Unsafe(mount_name).ReferencesParent())
     return std::string();
-  root.append(mount_name);
+  root.append(net::EscapePath(mount_name));
   root.append("/");
   return root;
 }