ucb: webdav-curl: handle additional HTTP status codes

Refactor a little to prevent recursive calls of ProcessRequest(),
to ensure the libcurl options set by the previous call are reset before
the following call.

Change-Id: Iba2ff695385c554058ac5a743c204224ca09ac57
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124059
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>

3 files changed, 144 insertions, 32 deletions

diff --git a/ucb/source/ucp/webdav-curl/CurlSession.cxx b/ucb/source/ucp/webdav-curl/CurlSession.cxx
index 371548cff234..2374ccbc26f2 100644
--- a/ucb/source/ucp/webdav-curl/CurlSession.cxx
+++ b/ucb/source/ucp/webdav-curl/CurlSession.cxx
@@ -116,13 +116,11 @@ public:
         , m_oGuard(::std::move(rFunc))
     {
     }
-#if 0
     void unlock()
     {
         m_oGuard.reset();
         m_Lock.unlock();
     }
-#endif
 };
 
 } // namespace
@@ -572,7 +570,7 @@ auto CurlSession::abort() -> void
 /// this is just a bunch of static member functions called from CurlSession
 struct CurlProcessor
 {
-    static auto ProcessRequest(
+    static auto ProcessRequestImpl(
         CurlSession& rSession, ::std::u16string_view rURIReference,
         DAVRequestEnvironment const* pEnv,
         ::std::unique_ptr<curl_slist, deleter_from_fn<curl_slist_free_all>> pRequestHeaderList,
@@ -580,14 +578,22 @@ struct CurlProcessor
         uno::Reference<io::XInputStream> const* pxInStream,
         ::std::pair<::std::vector<OUString> const&, DAVResource&> const* pRequestedHeaders) -> void;
 
+    static auto ProcessRequest(
+        Guard& rGuard, CurlSession& rSession, OUString const& rURIReference,
+        DAVRequestEnvironment const* pEnv,
+        ::std::unique_ptr<curl_slist, deleter_from_fn<curl_slist_free_all>> pRequestHeaderList,
+        uno::Reference<io::XOutputStream> const* pxOutStream,
+        uno::Reference<io::XInputStream> const* pxInStream,
+        ::std::pair<::std::vector<OUString> const&, DAVResource&> const* pRequestedHeaders) -> void;
+
     static auto
-    PropFind(CurlSession& rSession, ::std::u16string_view rURIReference, Depth depth,
+    PropFind(CurlSession& rSession, OUString const& rURIReference, Depth depth,
              ::std::tuple<::std::vector<OUString> const&, ::std::vector<DAVResource>* const,
                           ::std::vector<ucb::Lock>* const> const* o_pRequestedProperties,
              ::std::vector<DAVResourceInfo>* const o_pResourceInfos,
              DAVRequestEnvironment const& rEnv) -> void;
 
-    static auto MoveOrCopy(CurlSession& rSession, ::std::u16string_view rSourceURIReference,
+    static auto MoveOrCopy(CurlSession& rSession, OUString const& rSourceURIReference,
                            ::std::u16string_view rDestinationURI, DAVRequestEnvironment const& rEnv,
                            bool isOverwrite, char const* pMethod) -> void;
 
@@ -602,7 +608,7 @@ struct CurlProcessor
 };
 
 /// main function to initiate libcurl requests
-auto CurlProcessor::ProcessRequest(
+auto CurlProcessor::ProcessRequestImpl(
     CurlSession& rSession, ::std::u16string_view const rURIReference,
     DAVRequestEnvironment const* const pEnv,
     ::std::unique_ptr<curl_slist, deleter_from_fn<curl_slist_free_all>> pRequestHeaderList,
@@ -983,8 +989,108 @@ auto CurlProcessor::ProcessRequest(
     }
 }
 
+static auto TryRemoveExpiredLockToken(CurlSession& rSession, OUString const& rURIReference,
+                                      DAVRequestEnvironment const* const pEnv) -> bool
+{
+    if (!pEnv)
+    {
+        // caller was a NonInteractive_*LOCK function anyway, its caller is LockStore
+        return false;
+    }
+    OUString const* const pToken(g_Init.LockStore.getLockTokenForURI(rURIReference, nullptr));
+    if (!pToken)
+    {
+        return false;
+    }
+    try
+    {
+        // determine validity of existing lock via lockdiscovery request
+        ::std::vector<OUString> const propertyNames{ DAVProperties::LOCKDISCOVERY };
+        ::std::vector<ucb::Lock> locks;
+        ::std::tuple<::std::vector<OUString> const&, ::std::vector<DAVResource>* const,
+                     ::std::vector<ucb::Lock>* const> const args(propertyNames, nullptr, &locks);
+
+        CurlProcessor::PropFind(rSession, rURIReference, DAVZERO, &args, nullptr, *pEnv);
+
+        // https://datatracker.ietf.org/doc/html/rfc4918#section-15.8
+        // The response MAY not contain tokens, but hopefully it
+        // will if client is properly authenticated.
+        if (::std::any_of(locks.begin(), locks.end(), [pToken](ucb::Lock const& rLock) {
+                return ::std::any_of(
+                    rLock.LockTokens.begin(), rLock.LockTokens.end(),
+                    [pToken](OUString const& rToken) { return *pToken == rToken; });
+            }))
+        {
+            return false; // still have the lock
+        }
+
+        SAL_INFO("ucb.ucp.webdav.curl",
+                 "lock token expired, removing: " << rURIReference << " " << *pToken);
+        g_Init.LockStore.removeLock(rURIReference);
+        return true;
+    }
+    catch (DAVException const&)
+    {
+        return false; // ignore, the caller already has a better exception
+    }
+}
+
+auto CurlProcessor::ProcessRequest(
+    Guard& rGuard, CurlSession& rSession, OUString const& rURIReference,
+    DAVRequestEnvironment const* const pEnv,
+    ::std::unique_ptr<curl_slist, deleter_from_fn<curl_slist_free_all>> pRequestHeaderList,
+    uno::Reference<io::XOutputStream> const* const pxOutStream,
+    uno::Reference<io::XInputStream> const* const pxInStream,
+    ::std::pair<::std::vector<OUString> const&, DAVResource&> const* const pRequestedHeaders)
+    -> void
+{
+    try
+    {
+        ProcessRequestImpl(rSession, rURIReference, pEnv, ::std::move(pRequestHeaderList),
+                           pxOutStream, pxInStream, pRequestedHeaders);
+    }
+    catch (DAVException const& rException)
+    {
+        // error handling part 3: special HTTP status codes
+        if (rException.getError() == DAVException::DAV_HTTP_ERROR)
+        {
+            switch (rException.getStatus())
+            {
+                case SC_LOCKED:
+                {
+                    rGuard.unlock(); // release m_Mutex before accessing LockStore
+                    if (g_Init.LockStore.getLockTokenForURI(rURIReference, nullptr))
+                    {
+                        throw DAVException(DAVException::DAV_LOCKED_SELF);
+                    }
+                    else // locked by third party
+                    {
+                        throw DAVException(DAVException::DAV_LOCKED);
+                    }
+                    break;
+                }
+                case SC_PRECONDITION_FAILED:
+                case SC_BAD_REQUEST:
+                {
+                    rGuard.unlock(); // release m_Mutex before accessing LockStore
+                    // Not obvious but apparently these codes may indicate
+                    // the expiration of a lock.
+                    // Initiate a new request *outside* ProcessRequestImpl
+                    // *after* rGuard.unlock() to avoid messing up m_pCurl state.
+                    if (TryRemoveExpiredLockToken(rSession, rURIReference, pEnv))
+                    {
+                        throw DAVException(DAVException::DAV_LOCK_EXPIRED);
+                    }
+                    break;
+                }
+            }
+        }
+        throw; // everything else: re-throw
+    }
+}
+
 auto CurlProcessor::PropFind(
-    CurlSession& rSession, ::std::u16string_view const rURIReference, Depth const nDepth,
+    CurlSession& rSession, OUString const& rURIReference, Depth const nDepth,
     ::std::tuple<::std::vector<OUString> const&, ::std::vector<DAVResource>* const,
                  ::std::vector<ucb::Lock>* const> const* const o_pRequestedProperties,
     ::std::vector<DAVResourceInfo>* const o_pResourceInfos, DAVRequestEnvironment const& rEnv)
@@ -1090,7 +1196,7 @@ auto CurlProcessor::PropFind(
     assert(xResponseInStream.is());
     assert(xResponseOutStream.is());
 
-    CurlProcessor::ProcessRequest(rSession, rURIReference, &rEnv, ::std::move(pList),
+    CurlProcessor::ProcessRequest(g, rSession, rURIReference, &rEnv, ::std::move(pList),
                                   &xResponseOutStream, &xRequestInStream, nullptr);
 
     if (o_pResourceInfos)
@@ -1212,7 +1318,7 @@ auto CurlSession::PROPPATCH(OUString const& rURIReference,
     xWriter->endDocument();
     xRequestOutStream->closeOutput();
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, ::std::move(pList), nullptr,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, ::std::move(pList), nullptr,
                                   &xRequestInStream, nullptr);
 }
 
@@ -1233,7 +1339,8 @@ auto CurlSession::HEAD(OUString const& rURIReference, ::std::vector<OUString> co
     ::std::pair<::std::vector<OUString> const&, DAVResource&> const headers(rHeaderNames,
                                                                             io_rResource);
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, nullptr, nullptr, &headers);
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, nullptr, nullptr,
+                                  &headers);
 }
 
 auto CurlSession::GET(OUString const& rURIReference, DAVRequestEnvironment const& rEnv)
@@ -1260,7 +1367,7 @@ auto CurlSession::GET(OUString const& rURIReference, DAVRequestEnvironment const
     assert(rc == CURLE_OK);
     (void)rc;
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, &xResponseOutStream,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, &xResponseOutStream,
                                   nullptr, nullptr);
 
     uno::Reference<io::XInputStream> const xResponseInStream(
@@ -1285,7 +1392,7 @@ auto CurlSession::GET(OUString const& rURIReference, uno::Reference<io::XOutputS
     assert(rc == CURLE_OK);
     (void)rc;
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, &rxOutStream, nullptr,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, &rxOutStream, nullptr,
                                   nullptr);
 }
 
@@ -1312,7 +1419,7 @@ auto CurlSession::GET(OUString const& rURIReference, ::std::vector<OUString> con
     ::std::pair<::std::vector<OUString> const&, DAVResource&> const headers(rHeaderNames,
                                                                             io_rResource);
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, &xResponseOutStream,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, &xResponseOutStream,
                                   nullptr, &headers);
 
     uno::Reference<io::XInputStream> const xResponseInStream(
@@ -1341,7 +1448,7 @@ auto CurlSession::GET(OUString const& rURIReference, uno::Reference<io::XOutputS
     ::std::pair<::std::vector<OUString> const&, DAVResource&> const headers(rHeaderNames,
                                                                             io_rResource);
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, &rxOutStream, nullptr,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, &rxOutStream, nullptr,
                                   &headers);
 }
 
@@ -1374,7 +1481,7 @@ auto CurlSession::PUT(OUString const& rURIReference,
     // lock m_Mutex after accessing global LockStore to avoid deadlock
     Guard g(m_Mutex);
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, ::std::move(pList), nullptr,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, ::std::move(pList), nullptr,
                                   &rxInStream, nullptr);
 }
 
@@ -1420,7 +1527,7 @@ auto CurlSession::POST(OUString const& rURIReference, OUString const& rContentTy
     uno::Reference<io::XOutputStream> const xResponseOutStream(xSeqOutStream);
     assert(xResponseOutStream.is());
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, ::std::move(pList),
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, ::std::move(pList),
                                   &xResponseOutStream, &rxInStream, nullptr);
 
     uno::Reference<io::XInputStream> const xResponseInStream(
@@ -1469,7 +1576,7 @@ auto CurlSession::POST(OUString const& rURIReference, OUString const& rContentTy
     assert(rc == CURLE_OK);
     (void)rc;
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, ::std::move(pList), &rxOutStream,
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, ::std::move(pList), &rxOutStream,
                                   &rxInStream, nullptr);
 }
 
@@ -1490,11 +1597,11 @@ auto CurlSession::MKCOL(OUString const& rURIReference, DAVRequestEnvironment con
                            ConnectionEndPointString(m_URI.GetHost(), m_URI.GetPort()));
     }
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, nullptr, nullptr, nullptr);
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, nullptr, nullptr,
+                                  nullptr);
 }
 
-auto CurlProcessor::MoveOrCopy(CurlSession& rSession,
-                               ::std::u16string_view const rSourceURIReference,
+auto CurlProcessor::MoveOrCopy(CurlSession& rSession, OUString const& rSourceURIReference,
                                ::std::u16string_view const rDestinationURI,
                                DAVRequestEnvironment const& rEnv, bool const isOverwrite,
                                char const* const pMethod) -> void
@@ -1524,8 +1631,8 @@ auto CurlProcessor::MoveOrCopy(CurlSession& rSession,
     assert(rc == CURLE_OK);
     (void)rc;
 
-    CurlProcessor::ProcessRequest(rSession, rSourceURIReference, &rEnv, ::std::move(pList), nullptr,
-                                  nullptr, nullptr);
+    CurlProcessor::ProcessRequest(g, rSession, rSourceURIReference, &rEnv, ::std::move(pList),
+                                  nullptr, nullptr, nullptr);
 }
 
 auto CurlSession::COPY(OUString const& rSourceURIReference, OUString const& rDestinationURI,
@@ -1563,7 +1670,8 @@ auto CurlSession::DESTROY(OUString const& rURIReference, DAVRequestEnvironment c
                            ConnectionEndPointString(m_URI.GetHost(), m_URI.GetPort()));
     }
 
-    CurlProcessor::ProcessRequest(*this, rURIReference, &rEnv, nullptr, nullptr, nullptr, nullptr);
+    CurlProcessor::ProcessRequest(g, *this, rURIReference, &rEnv, nullptr, nullptr, nullptr,
+                                  nullptr);
 }
 
 auto CurlProcessor::Lock(
@@ -1595,7 +1703,7 @@ auto CurlProcessor::Lock(
     TimeValue startTime;
     osl_getSystemTime(&startTime);
 
-    CurlProcessor::ProcessRequest(rSession, rURIReference, pEnv, ::std::move(pRequestHeaderList),
+    CurlProcessor::ProcessRequest(g, rSession, rURIReference, pEnv, ::std::move(pRequestHeaderList),
                                   &xResponseOutStream, pxRequestInStream, nullptr);
 
     ::std::vector<ucb::Lock> const acquiredLocks(parseWebDAVLockResponse(xResponseInStream));
@@ -1639,7 +1747,7 @@ auto CurlSession::LOCK(OUString const& rURIReference, ucb::Lock /*const*/& rLock
     SAL_INFO("ucb.ucp.webdav.curl", "LOCK: " << rURIReference);
 
     // FIXME: why is a *global* LockStore keyed by *path*?
-    if (g_Init.LockStore.getLockTokenForURI(rURIReference, rLock))
+    if (g_Init.LockStore.getLockTokenForURI(rURIReference, &rLock))
     {
         // already have a lock that covers the requirement
         // TODO: maybe use DAV:lockdiscovery to ensure it's valid
@@ -1786,7 +1894,7 @@ auto CurlProcessor::Unlock(CurlSession& rSession, OUString const& rURIReference,
             ConnectionEndPointString(rSession.m_URI.GetHost(), rSession.m_URI.GetPort()));
     }
 
-    CurlProcessor::ProcessRequest(rSession, rURIReference, pEnv, ::std::move(pList), nullptr,
+    CurlProcessor::ProcessRequest(g, rSession, rURIReference, pEnv, ::std::move(pList), nullptr,
                                   nullptr, nullptr);
 }
 
diff --git a/ucb/source/ucp/webdav-curl/SerfLockStore.cxx b/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
index cae43864f8e5..10436ea969e1 100644
--- a/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
+++ b/ucb/source/ucp/webdav-curl/SerfLockStore.cxx
@@ -144,7 +144,7 @@ OUString SerfLockStore::getLockToken( const OUString& rLock )
 }
 
 OUString const*
-SerfLockStore::getLockTokenForURI(OUString const& rURI, css::ucb::Lock const& rLock)
+SerfLockStore::getLockTokenForURI(OUString const& rURI, css::ucb::Lock const*const pLock)
 {
     osl::MutexGuard aGuard( m_aMutex );
 
@@ -154,17 +154,21 @@ SerfLockStore::getLockTokenForURI(OUString const& rURI, css::ucb::Lock const& rL
     {
         return nullptr;
     }
+    if (!pLock) // any lock will do
+    {
+        return &it->second.m_sToken;
+    }
     // 0: EXCLUSIVE 1: SHARED
-    if (it->second.m_Lock.Scope == ucb::LockScope_SHARED && rLock.Scope == ucb::LockScope_EXCLUSIVE)
+    if (it->second.m_Lock.Scope == ucb::LockScope_SHARED && pLock->Scope == ucb::LockScope_EXCLUSIVE)
     {
         return nullptr;
     }
-    assert(it->second.m_Lock.Type == rLock.Type); // only WRITE possible
-    if (it->second.m_Lock.Depth < rLock.Depth)
+    assert(it->second.m_Lock.Type == pLock->Type); // only WRITE possible
+    if (it->second.m_Lock.Depth < pLock->Depth)
     {
         return nullptr;
     }
-    assert(it->second.m_Lock.Owner == rLock.Owner); // only own locks expected
+    assert(it->second.m_Lock.Owner == pLock->Owner); // only own locks expected
     // ignore Timeout ?
     return &it->second.m_sToken;
 }
diff --git a/ucb/source/ucp/webdav-curl/SerfLockStore.hxx b/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
index c7d1499a6e65..f87516a2fa2e 100644
--- a/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
+++ b/ucb/source/ucp/webdav-curl/SerfLockStore.hxx
@@ -70,7 +70,7 @@ public:
     bool finishing() const;
     OUString getLockToken( const OUString& rLock );
 
-    OUString const* getLockTokenForURI(OUString const& rURI, css::ucb::Lock const& rLock);
+    OUString const* getLockTokenForURI(OUString const& rURI, css::ucb::Lock const* pLock);
 
     void addLock( const OUString& rURI,
                   css::ucb::Lock const& rLock,