While constructing FileAPI URLs special charactes are not escaped and the URL gets invalid. Outside Chromium this issue has been fixed in https://bugs.webkit.org/show_bug.cgi?id=60218.
Created attachment 101871 [details] Fix that escapes filenames in KURLGoogle.cpp (KURL.cpp already do it).
Attachment 101871 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/filesystem/op-get-entry-e..." exit_code: 1 Source/WebCore/platform/KURLGoogle.cpp:792: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 101981 [details] Fixed code style
Comment on attachment 101981 [details] Fixed code style Attachment 101981 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9244396
Comment on attachment 101981 [details] Fixed code style Attachment 101981 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9247362
Comment on attachment 101981 [details] Fixed code style Attachment 101981 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9248325
Created attachment 101986 [details] Removed accidentally included files.
Comment on attachment 101986 [details] Removed accidentally included files. View in context: https://bugs.webkit.org/attachment.cgi?id=101986&action=review > Source/WebCore/platform/KURLGoogle.cpp:769 > + const UChar* pos = path.characters(); KURLGoogle is meant to just be a lightweight adapter to the google-url library. I suspect the proper fix should be a patch to google-url instead, but I defer to Brett Wilson's judgement on that.
setPath is definitely the wrong place for this. What happens when somebody constructs a full URL without going through setPath? You'll get different canonicalization behavior. I presume your file refs are going through the "path URL" case of googleurl because you're using some random scheme? In this case, how much modification of these URLs do you need? In the case of blob URLs, we decided not to add support to googleurl for them. In this way, they're like data URLs. Data URLs have a specific format & requirements, but it's not googleurl's (or KURL's) job to interpret it, it's the code that's generating or interpreting the data URL's job to make sure it's well-formed & escaped properly. If your file stuff is similar, then your component should be generating properly escaped paths and we shouldn't need this. On the other hand, if we're going to be doing a lot of stuff, like doing relative URL resolution and fragment navigation on these URLs, it would be better to teach googleurl about them so the normal loader behavior works as expected.
File URLs have the format filesystem:http://webkit.org/temporary/path/to/file.png They should support resolving relative URLs, query strings, and hash fragments.
WebKit's KURL::setPath is supposed to work properly with any argument including having '#' and '?' symbols (AsyncFileSystemChromium::virtualPathToFileSystemURL relies on it). Google's url_util::ReplaceComponents apparently is't. Consequently adaptor's code (that's the KURLGoogle.ccp, KURL::setPath or KURLGooglePrivate::replaceComponents) looks to be the right place to handle the difference. As Darin said the scheme is filesystem:[securuty-origin]/[path]. [path] is not as restricted as in the BLOB scheme. Instead of forcing googleurl to escaping all input of url_util::ReplaceComponents I would suggest to use encodeWithURLEscapeSequences/decodeURLEscapeSequences in each KURL getter and setter of KURLGoogle.cpp whose KURL.cpp counterpart does. (In reply to comment #9) > setPath is definitely the wrong place for this. What happens when somebody constructs a full URL without going through setPath? You'll get different canonicalization behavior. > > I presume your file refs are going through the "path URL" case of googleurl because you're using some random scheme? In this case, how much modification of these URLs do you need? > > In the case of blob URLs, we decided not to add support to googleurl for them. In this way, they're like data URLs. Data URLs have a specific format & requirements, but it's not googleurl's (or KURL's) job to interpret it, it's the code that's generating or interpreting the data URL's job to make sure it's well-formed & escaped properly. > > If your file stuff is similar, then your component should be generating properly escaped paths and we shouldn't need this. On the other hand, if we're going to be doing a lot of stuff, like doing relative URL resolution and fragment navigation on these URLs, it would be better to teach googleurl about them so the normal loader behavior works as expected.
ReplaceComponents does the appropriate escaping for the type of URL you have. If you have an http URL, it will escape the ? and #. The kind of URL you have (a path URL) doesn't have ? or # components, so it doesn't escape them. We need to teach googleurl about these FileAPI files for this reason, and for relative URL resolution problems as well. When this is done, it shouldn't be necessary to make any changes to setPath.
(In reply to comment #12) > ReplaceComponents does the appropriate escaping for the type of URL you have. If you have an http URL, it will escape the ? and #. The kind of URL you have (a path URL) doesn't have ? or # components, so it doesn't escape them. > > We need to teach googleurl about these FileAPI files for this reason, and for relative URL resolution problems as well. When this is done, it shouldn't be necessary to make any changes to setPath. I'm deep into the GURL code working on this right now [and all the chromium code that calls it], but we will still need to add escaping to EntryBase::toURL() in EntryBase.cpp.
Well, WebKit's KURL does escaping but it's not necessary required. Escaping needed because of the way such an URL is parsed to extract a file path. In Chromium everything after '#' goes into the hash part (not into the 'path'). But as far as I understand the filesystem URL scheme is not supposed to have that part.
Comment on attachment 101986 [details] Removed accidentally included files. I know that this needs a ChangeLog before it can be committed (but I don't know about the change itself). Probably best to ping brettw about that if more input is needed.
(In reply to comment #14) > Well, WebKit's KURL does escaping but it's not necessary required. Escaping needed because of the way such an URL is parsed to extract a file path. In Chromium everything after '#' goes into the hash part (not into the 'path'). But as far as I understand the filesystem URL scheme is not supposed to have that part. Actually, filesystem URLs are supposed to have # and ? parts--they just don't because I haven't finished the work on GURL yet.
https://code.google.com/p/chromium/issues/detail?id=230594