From 687788e4d30bf36dd93c39d72335f331f49f0745 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 4 Jan 2021 14:22:19 +0100 Subject: [PATCH 1/4] Fix Webdav storage PROPFIND request Remove additional "a:prop" in PROPFIND request to match RFC 4918 section 9.1.3. Added Content-Type header as the body is not a true multipart POST. Signed-off-by: Vincent Petry --- src/storage/plugins/CurlStorage.cxx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index ccfb0bf12..ad6f71fce 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -262,18 +262,19 @@ public: request.SetOption(CURLOPT_MAXREDIRS, 1L); request_headers.Append(StringFormat<40>("depth: %u", depth)); + request_headers.Append("content-type: text/xml"); request.SetOption(CURLOPT_HTTPHEADER, request_headers.Get()); request.SetOption(CURLOPT_POSTFIELDS, "\n" "" - "" - "" - "" + "" + "" + "" + "" + "" ""); - - // TODO: send request body } using BlockingHttpRequest::GetEasy; From b7d0001390766127cb51b7e12b5765cfc04ed807 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 4 Jan 2021 15:21:53 +0100 Subject: [PATCH 2/4] Fix parsing propstat blocks There can be more than one propstat block each with their own status code. We're only interested in the one with the 200 status, the found properties. This fixes parsing to make sure we process all propstat blocks instead of just the last one, which might have a 404 status for not-found properties. Signed-off-by: Vincent Petry --- src/storage/plugins/CurlStorage.cxx | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index ad6f71fce..c986a8a49 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -243,6 +243,7 @@ class PropfindOperation : BlockingHttpRequest, CommonExpatParser { enum class State { ROOT, RESPONSE, + PROPSTAT, HREF, STATUS, TYPE, @@ -322,9 +323,13 @@ private: break; case State::RESPONSE: - if (strcmp(name, "DAV:|href") == 0) + if (strcmp(name, "DAV:|propstat") == 0) + state = State::PROPSTAT; + else if (strcmp(name, "DAV:|href") == 0) state = State::HREF; - else if (strcmp(name, "DAV:|status") == 0) + break; + case State::PROPSTAT: + if (strcmp(name, "DAV:|status") == 0) state = State::STATUS; else if (strcmp(name, "DAV:|resourcetype") == 0) state = State::TYPE; @@ -354,9 +359,15 @@ private: case State::RESPONSE: if (strcmp(name, "DAV:|response") == 0) { - FinishResponse(); state = State::ROOT; } + break; + + case State::PROPSTAT: + if (strcmp(name, "DAV:|propstat") == 0) { + FinishResponse(); + state = State::RESPONSE; + } break; @@ -367,22 +378,22 @@ private: case State::STATUS: if (strcmp(name, "DAV:|status") == 0) - state = State::RESPONSE; + state = State::PROPSTAT; break; case State::TYPE: if (strcmp(name, "DAV:|resourcetype") == 0) - state = State::RESPONSE; + state = State::PROPSTAT; break; case State::MTIME: if (strcmp(name, "DAV:|getlastmodified") == 0) - state = State::RESPONSE; + state = State::PROPSTAT; break; case State::LENGTH: if (strcmp(name, "DAV:|getcontentlength") == 0) - state = State::RESPONSE; + state = State::PROPSTAT; break; } } @@ -390,6 +401,7 @@ private: void CharacterData(const XML_Char *s, int len) final { switch (state) { case State::ROOT: + case State::PROPSTAT: case State::RESPONSE: case State::TYPE: break; From 216f62ea1468933f4a78f17885b27e37e1393d8c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 4 Jan 2021 21:50:03 +0100 Subject: [PATCH 3/4] Webdav href in response can be relative Fixed Webdav base path stripping in cases where href is a relative path. Signed-off-by: Vincent Petry --- src/storage/plugins/CurlStorage.cxx | 35 +++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index c986a8a49..298d70967 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -468,11 +468,19 @@ CurlStorage::GetInfo(std::string_view uri_utf8, [[maybe_unused]] bool follow) gcc_pure static std::string_view -UriPathOrSlash(const char *uri) noexcept +UriPathOrSlash(const char *uri, bool relative) noexcept { auto path = uri_get_path(uri); if (path.data() == nullptr) path = "/"; + else if (relative) { + // search after first slash + path = path.substr(1); + auto slash = path.find('/'); + if (slash != std::string_view::npos) + path = path.substr(slash); + } + return path; } @@ -481,13 +489,15 @@ UriPathOrSlash(const char *uri) noexcept */ class HttpListDirectoryOperation final : public PropfindOperation { const std::string base_path; + const std::string base_path_relative; MemoryStorageDirectoryReader::List entries; public: HttpListDirectoryOperation(CurlGlobal &curl, const char *uri) :PropfindOperation(curl, uri, 1), - base_path(CurlUnescape(GetEasy(), UriPathOrSlash(uri))) {} + base_path(CurlUnescape(GetEasy(), UriPathOrSlash(uri, false))), + base_path_relative(CurlUnescape(GetEasy(), UriPathOrSlash(uri, true))) {} std::unique_ptr Perform() { DeferStart(); @@ -506,6 +516,7 @@ private: */ gcc_pure StringView HrefToEscapedName(const char *href) const noexcept { + StringView relative_path; StringView path = uri_get_path(href); if (path == nullptr) return nullptr; @@ -513,17 +524,23 @@ private: /* kludge: ignoring case in this comparison to avoid false negatives if the web server uses a different case */ - path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); - if (path == nullptr || path.empty()) - return nullptr; + relative_path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); + if (relative_path == nullptr || relative_path.empty()) { + // try relative base path + relative_path = StringAfterPrefixIgnoreCase(path, base_path_relative.c_str()); + } - const char *slash = path.Find('/'); + if (relative_path == nullptr || relative_path.empty()) { + return nullptr; + } + + const char *slash = relative_path.Find('/'); if (slash == nullptr) /* regular file */ - return path; - else if (slash == &path.back()) + return relative_path; + else if (slash == &relative_path.back()) /* trailing slash: collection; strip the slash */ - return {path.data, slash}; + return {relative_path.data, slash}; else /* strange, better ignore it */ return nullptr; From 74b2fc7fdca9be13cbbe4cb52b2fab573b3cf82c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 5 Jan 2021 12:04:08 +0100 Subject: [PATCH 4/4] Use uri_has_scheme for Webdav response href Use uri_has_scheme to find out if the href in Webdav responses is absolute to use the matching base path extraction. Signed-off-by: Vincent Petry --- src/storage/plugins/CurlStorage.cxx | 19 +++++++++---------- src/util/UriExtract.cxx | 2 +- src/util/UriExtract.hxx | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/storage/plugins/CurlStorage.cxx b/src/storage/plugins/CurlStorage.cxx index 298d70967..d717a824c 100644 --- a/src/storage/plugins/CurlStorage.cxx +++ b/src/storage/plugins/CurlStorage.cxx @@ -516,7 +516,6 @@ private: */ gcc_pure StringView HrefToEscapedName(const char *href) const noexcept { - StringView relative_path; StringView path = uri_get_path(href); if (path == nullptr) return nullptr; @@ -524,23 +523,23 @@ private: /* kludge: ignoring case in this comparison to avoid false negatives if the web server uses a different case */ - relative_path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); - if (relative_path == nullptr || relative_path.empty()) { - // try relative base path - relative_path = StringAfterPrefixIgnoreCase(path, base_path_relative.c_str()); + if (uri_has_scheme(path)) { + path = StringAfterPrefixIgnoreCase(path, base_path.c_str()); + } else { + path = StringAfterPrefixIgnoreCase(path, base_path_relative.c_str()); } - if (relative_path == nullptr || relative_path.empty()) { + if (path == nullptr || path.empty()) { return nullptr; } - const char *slash = relative_path.Find('/'); + const char *slash = path.Find('/'); if (slash == nullptr) /* regular file */ - return relative_path; - else if (slash == &relative_path.back()) + return path; + else if (slash == &path.back()) /* trailing slash: collection; strip the slash */ - return {relative_path.data, slash}; + return {path.data, slash}; else /* strange, better ignore it */ return nullptr; diff --git a/src/util/UriExtract.cxx b/src/util/UriExtract.cxx index 053c20cc7..ab8a7244a 100644 --- a/src/util/UriExtract.cxx +++ b/src/util/UriExtract.cxx @@ -85,7 +85,7 @@ uri_after_scheme(std::string_view uri) noexcept } bool -uri_has_scheme(const char *uri) noexcept +uri_has_scheme(std::string_view uri) noexcept { return !uri_get_scheme(uri).empty(); } diff --git a/src/util/UriExtract.hxx b/src/util/UriExtract.hxx index 05e288b30..5f17910ea 100644 --- a/src/util/UriExtract.hxx +++ b/src/util/UriExtract.hxx @@ -40,7 +40,7 @@ */ gcc_pure bool -uri_has_scheme(const char *uri) noexcept; +uri_has_scheme(std::string_view uri) noexcept; /** * Returns the scheme name of the specified URI, or an empty string.