ucb/source/ucp/webdav-curl/CurlSession.cxx | 160 ++++++++++++++++++++++-----
ucb/source/ucp/webdav-curl/SerfLockStore.cxx | 14 +-
ucb/source/ucp/webdav-curl/SerfLockStore.hxx | 2
3 files changed, 144 insertions(+), 32 deletions(-)
New commits:
commit 33fb5accfa38fa6ae739ed20d807bf1980eb4b59
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Oct 21 13:54:23 2021 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Nov 1 18:18:01 2021 +0100
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.st...@allotropia.de>
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,
