Consider the usage of WebDAV methods sufficient for assuming WebDAV
ClosedPublic

Authored by vkrause on Sep 30 2019, 5:14 PM.

Details

Summary

So far WebDAV only really works when using webdav[s]: URLs, which is an
unexpected KIO-specific detail that does not seem to be understood by all
applications (see e.g. the hacks in KDAV::DavManager). With this change
at least the WebDAV methods also emit the WebDAV specific headers when
using http[s] URLs.

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.
vkrause created this revision.Sep 30 2019, 5:14 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 30 2019, 5:14 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
vkrause requested review of this revision.Sep 30 2019, 5:14 PM

Test Plan: ?

Did you see this in a specific application? Just to have some context about why/when this occurs.

dfaure accepted this revision.Sep 30 2019, 6:30 PM

Ah I see, the hacks in KDAV::DavManager is the context :)

This revision is now accepted and ready to land.Sep 30 2019, 6:30 PM
This revision was automatically updated to reflect the committed changes.

This commit completely breaks the davgroupware resource for me (to access kolab calendars via CALDAV).

REPORT requests get a 400 Bad Request error from the server.

2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: ============ Sending Header:
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "REPORT /calendars/dfaure@kdab.com/0a9636a30e83-8c02805b740e-0a0dee75/ HTTP/1.1"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Host: caldav.kdab.com"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Connection: keep-alive"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/534.34 (KHTML, like Gecko) akonadi_davgroupware_resource_11/5.12.2 Safari/534.34"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Pragma: no-cache"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Cache-control: no-cache"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Accept: text/html, text/*;q=0.9, image/jpeg;q=0.9, image/png;q=0.9, image/*;q=0.9, */*;q=0.8"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Accept-Encoding: gzip, deflate, x-gzip, x-deflate"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Accept-Charset: utf-8,*;q=0.5"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Accept-Language: en-US,en;q=0.9"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Content-Type: text/xml"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Depth: 1"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Authorization: Basic [snipped"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Depth: 1"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendQuery: "Content-Type: text/xml; charset=utf-8"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::sendBody: "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n<calendar-query xmlns=\"urn:ietf:params:xml:ns:caldav\">\n <prop xmlns=\"DAV:\">\n <getetag xmlns=\"DAV:\"/>\n <resourcetype xmlns=\"DAV:\"/>\n </prop>\n <filter xmlns=\"urn:ietf:params:xml:ns:caldav\">\n <comp-filter xmlns=\"urn:ietf:params:xml:ns:caldav\" name=\"VCALENDAR\">\n <comp-filter xmlns=\"urn:ietf:params:xml:ns:caldav\" name=\"VEVENT\"/>\n </comp-filter>\n </filter>\n</calendar-query>"
2019-10-22T12:35:15 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::readResponseHeader:
2019-10-22T12:35:17 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::readResponseHeader: ============ Received Status Response:
2019-10-22T12:35:17 kio_http(12004)/kf5.kio.kio_http HTTPProtocol::readResponseHeader: "HTTP/1.1 400 Bad Request"

I think the problem is that the request has two Content-Type headers. The second one is added by the code we are now entering, line 2603, because of the modified if().

Depth: 1 is duplicated too.

I tried adding a contentType.isEmpty() check, but it is actually empty. Needs more debugging....

Which version of KDAV do you have? master or some older release?

Overall we probably have two options, revert this change (and the corresponding change in KDAV) and defer it to KF6, or harden this code further against duplicates in custom headers. The below might work for Content-Type (no way to test here atm), a similar fix for Depth would require a bit more code reshuffling I think.

diff --git a/src/ioslaves/http/http.cpp b/src/ioslaves/http/http.cpp
index e3bad6cc..88c13999 100644
--- a/src/ioslaves/http/http.cpp
+++ b/src/ioslaves/http/http.cpp
@@ -2599,7 +2599,7 @@ bool HTTPProtocol::sendQuery()
             davHeader += metaData(QStringLiteral("davHeader"));
 
             // Set content type of webdav data
-            if (hasDavData) {
+            if (hasDavData && !header.contains(QLatin1String("Content-Type: "))) {
                 davHeader += QStringLiteral("Content-Type: text/xml; charset=utf-8\r\n");
             }

I'm on the Applications/19.08 git branch, which shouldn't get broken by a newer KF5 release.

Your patch works for Content-Type, but the duplicated Depth still leads to "Bad request".

Where does the second Depth come from? I see one at line 2411, I guess the other comes from kdav via metadata, but not from line 2599 according to my debug output.

I assume that if we revert this (or rather #ifdef KF6), kdav master needs a revert (#ifdef) too?

I'm on the Applications/19.08 git branch, which shouldn't get broken by a newer KF5 release.

Your patch works for Content-Type, but the duplicated Depth still leads to "Bad request".

Where does the second Depth come from? I see one at line 2411, I guess the other comes from kdav via metadata, but not from line 2599 according to my debug output.

The second Depth is likely also in customHTTPHeader meta data, which currently isn't available for checking when generating the Depth header. Moving this further up and doing a similar check in 2411 might be enough.

I assume that if we revert this (or rather #ifdef KF6), kdav master needs a revert (#ifdef) too?

Yes.

Can I just ignore the Depth from the custom http headers instead?

--- i/src/ioslaves/http/http.cpp
+++ w/src/ioslaves/http/http.cpp
@@ -172,7 +172,8 @@ static QString sanitizeCustomHTTPHeader(const QString &_header)
         if (!header.contains(QLatin1Char(':')) ||
                 header.startsWith(QLatin1String("host"), Qt::CaseInsensitive) ||
                 header.startsWith(QLatin1String("proxy-authorization"), Qt::CaseInsensitive) ||
-                header.startsWith(QLatin1String("via"), Qt::CaseInsensitive)) {
+                header.startsWith(QLatin1String("via"), Qt::CaseInsensitive) ||
+                header.startsWith(QLatin1String("depth"), Qt::CaseInsensitive)) {
             continue;
         }

Can I just ignore the Depth from the custom http headers instead?

--- i/src/ioslaves/http/http.cpp
+++ w/src/ioslaves/http/http.cpp
@@ -172,7 +172,8 @@ static QString sanitizeCustomHTTPHeader(const QString &_header)
         if (!header.contains(QLatin1Char(':')) ||
                 header.startsWith(QLatin1String("host"), Qt::CaseInsensitive) ||
                 header.startsWith(QLatin1String("proxy-authorization"), Qt::CaseInsensitive) ||
-                header.startsWith(QLatin1String("via"), Qt::CaseInsensitive)) {
+                header.startsWith(QLatin1String("via"), Qt::CaseInsensitive) ||
+                header.startsWith(QLatin1String("depth"), Qt::CaseInsensitive)) {
             continue;
         }

Indeed, that should work too, especially considering Depth has its own meta-data field anyway.

meven added a subscriber: meven.Oct 24 2019, 5:11 PM

The new bug https://bugs.kde.org/show_bug.cgi?id=413117 mentions this as well :

kio-5.63 makes kdav-19.08.2 fail with HTTP error (400)

STEPS TO REPRODUCE
Install the mentioned packages. Try to connect to a davical server via https.

RESULT
HTTP error (400).

Works with kio-5.62.

Reverting commit 9713ea02e49eda11d72e1ac605417dac0dab8c37
"Consider the usage of WebDAV methods sufficient for assuming WebDAV" in kio-5.63 makes kdav work again.

I forgot to link the fix here: D24880