Fix WebDAV destination header on COPY and MOVE operations
ClosedPublic

Authored by dantti on Jul 9 2018, 3:07 PM.

Details

Summary

The Destination header must always be QUrl::FullyEncoded
so that special characters can be used as destination.

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dantti created this revision.Jul 9 2018, 3:07 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJul 9 2018, 3:07 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
dantti requested review of this revision.Jul 9 2018, 3:07 PM
dantti added a comment.Sep 8 2018, 2:43 PM

Ping? This is a pretty obvious fix, desturl is QString which later becomes latin1 loosing characters...

bruns added a subscriber: bruns.Sep 8 2018, 8:11 PM

Ping? This is a pretty obvious fix, desturl is QString which later becomes latin1 loosing characters...

Can you add a test plan? Makes it much more obvious how to replicate the problem.

dantti added a comment.Sep 9 2018, 5:40 AM

Test plan:

  1. Connecto to a WebDav server
  2. Create a file named "foo"
  3. Try to rename the file to "speçiál" - it should result in "spe"

The COPY operation has the same issue but I'm not sure how to test it right now,
probably it will also fail if the destination directory is named with special characters (non-latin1).

bruns added a comment.Sep 9 2018, 1:17 PM

Test plan:

  1. Connecto to a WebDav server
  2. Create a file named "foo"
  3. Try to rename the file to "speçiál" - it should result in "spe"

    The COPY operation has the same issue but I'm not sure how to test it right now, probably it will also fail if the destination directory is named with special characters (non-latin1).

The test plan should be part of the commit, use "Edit revision" above.

The 3rd point is strange - it should of course result in "speçiál", everything else would be silent data corruption.

dantti added a comment.Sep 9 2018, 3:05 PM

Well the issue will probably depend on the server,
for example Apache2 with Nextcloud it returns forbidden for both operations,
in Cloudlyst which does:
QUrl::fromEncoded(QStringLiteral("file:///speçiál").toLatin1()).toLocalFile();
you get a file with a different file name as URL.
Since 'ç' is extended ASCII this "fails" only because URL are sopposed
to use only printable ASCII chars

If you use some some UTF-8 char like ←it wil
produce that behavior:
QStringLiteral("spe←ail\r\n").toLatin1()
will produce:
"spe?ail\r\n"
and the question mark is interpreted as being the query part
of an URL, thus what I said before that it cuts
everything past the special char due URL parsing.
you can see doing this:
QUrl::fromEncoded(QStringLiteral("file:///file←sadssad").toLatin1()).toLocalFile()
local file will be "/file"

dfaure accepted this revision.Sep 17 2018, 5:11 PM

FullyEncoded makes perfect sense for sending in a protocol implementation.

This revision is now accepted and ready to land.Sep 17 2018, 5:11 PM
This revision was automatically updated to reflect the committed changes.