Differential D13691

Use percent encoding when creating urls for UDS entries
ClosedPublic
Actions

Authored by elvisangelaccio on Jun 23 2018, 9:05 AM.

Details

Reviewers
dvratil
dfaure
Commits
R219:98eac3003a07: Use percent encoding when creating urls for UDS entries
Summary

Otherwise it won't be possible to open gdrive files that contain special
characters (such as #) in their path.

This is not handled by libkgapi, which just copies whatever is in the
JSON from Google, and Google does not seem to encode the filenames
either. So we encode the filenames at the kio-gdrive level, where we
actually play with QUrls.

We do so by actually creating a QUrl (which takes care of
percent-encoding in setPath()) and then we convert it to a string.

While at it, use faster concatenation rather than arg parsing.

BUG: 395590
FIXED-IN: 1.2.4

Test Plan

Try to open a file with # in the name.
Check whether files with normal names still work.

Diff Detail

Repository
R219 KIO GDrive
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
elvisangelaccio requested review of this revision.Jun 23 2018, 9:05 AM
elvisangelaccio created this revision.
dfaure added a comment.Jun 27 2018, 7:04 AM

String concatenation (or arg, which is the same) clearly isn't the right way to create URLs, as shown by this problem.
The better solution is

QUrl url;
url.setScheme("gdrive");
url.setHost(path);
url.setPath(origFile->title());
url.setQuery(origFile->id());
entry.insert(KIO::UDSEntry::UDS_URL, url.toString());

elvisangelaccio updated this revision to Diff 36862.Jun 28 2018, 9:51 PM
  • Actually build a QUrl, which takes care of percent-encoding for free.
elvisangelaccio added a comment.Jun 28 2018, 9:52 PM

@dfaure Done, thanks for the hint (btw we don't have the host in gdrive urls, I hope that's ok)

dfaure accepted this revision.EditedJun 29 2018, 7:51 AM

Ah yes of course, I missed the fact that the replacement for %1 starts with a '/', so we ended up with ///path, not //path.

src/kio_gdrive.cpp
230

If this code is performance critical (i.e. when listing a folder with many many files), I'd use path + '/' + file->title().
arg() is obviously slower since it has to be parsed for %1 and %2.

This revision is now accepted and ready to land.Jun 29 2018, 7:51 AM
elvisangelaccio updated this revision to Diff 36870.Jun 29 2018, 8:45 AM
elvisangelaccio marked an inline comment as done.
  • Use faster concatenation
elvisangelaccio edited the summary of this revision. (Show Details)Jun 29 2018, 8:47 AM
dfaure accepted this revision.Jun 29 2018, 9:07 AM
Closed by commit R219:98eac3003a07: Use percent encoding when creating urls for UDS entries (authored by elvisangelaccio). · Explain WhyJun 29 2018, 9:09 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mautotests/urltest.cpp (29 lines)
Msrc/kio_gdrive.h (1 line)
Msrc/kio_gdrive.cpp (16 lines)

Diff 36872

View Options

autotests/urltest.cpp

Loading...
View Options

src/kio_gdrive.h

Loading...
View Options

src/kio_gdrive.cpp

Loading...