ucb/source/ucp/webdav-curl/webdavcontent.cxx | 286 ++++++++++++++++++-----
ucb/source/ucp/webdav-curl/webdavcontent.hxx | 12
ucb/source/ucp/webdav-curl/webdavcontentcaps.cxx | 3
3 files changed, 236 insertions(+), 65 deletions(-)
New commits:
commit b5d91ff10fda72bc9758e7087f8a1a24d2d27022
Author: Giuseppe Castagno <giuseppe.casta...@acca-esse.eu>
AuthorDate: Fri Aug 28 18:52:36 2015 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Nov 1 18:26:14 2021 +0100
ucb: webdav-curl: tdf#82744: fix WebDAV lock/unlock behaviour - part 3
Changes done to the code in sfx2, ucbhelper, ucb, unotools in no particular
order
- add method helpers to call the ucb lock/unlock
- add lock/unlock 'real' management
- make DateChange property retrieval working for WebDAV as well
- add check for changed content of a WebDAV file, in order to reload
it correctly when 'Edit Mode' command is activated from GUI
- Unlock WebDAV file while saving only if explicitly enabled
Needed in order to avoid the small window of file unlocked state that
opens while saving a file.
When saving LO actually does as follows:
- unlock the prevoius version of the file
- prepares operations to save the modified version
- lock the new file
- save the new version
- the lock method is enabled if the DAV resource supports it.
In case the lock is not supported, for example example DAV with lock
disabled, the lock method is disabled.
Exception: when the resource is first created and the lock is not
supported: a lock command is sent anyway, because if the resource is not
yet present, there is no method to detect the lock/unlock availability
in this case.
- cppcheck:noExplicitConstructor
[ port of commit b4576f3da4d90139fc5140962d13cb91dab98797
excluding the obsolete FTP scheme thing ]
Change-Id: I16bb4e2fa9899fd31af7c223390f3fb213330fa4
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123228
Tested-by: Michael Stahl <michael.st...@allotropia.de>
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
diff --git a/ucb/source/ucp/webdav-curl/webdavcontent.cxx
b/ucb/source/ucp/webdav-curl/webdavcontent.cxx
index d0756cdb634d..f9933c639074 100644
--- a/ucb/source/ucp/webdav-curl/webdavcontent.cxx
+++ b/ucb/source/ucp/webdav-curl/webdavcontent.cxx
@@ -197,7 +197,9 @@ Content::Content(
rtl::Reference< DAVSessionFactory > const & rSessionFactory )
: ContentImplHelper( rxContext, pProvider, Identifier ),
m_eResourceType( UNKNOWN ),
+ m_eResourceTypeForLocks( UNKNOWN ),
m_pProvider( pProvider ),
+ m_rSessionFactory( rSessionFactory ),
m_bTransient( false ),
m_bCollection( false ),
m_bDidGetOrHead( false )
@@ -228,6 +230,7 @@ Content::Content(
bool isCollection )
: ContentImplHelper( rxContext, pProvider, Identifier ),
m_eResourceType( UNKNOWN ),
+ m_eResourceTypeForLocks( UNKNOWN ),
m_pProvider( pProvider ),
m_bTransient( true ),
m_bCollection( isCollection ),
@@ -635,23 +638,28 @@ uno::Any SAL_CALL Content::execute(
// lock
- // supportsExclusiveWriteLock() does not work if the file is being
created.
- // The lack of lock functionality is taken care of inside lock(),
- // a temporary measure.
- // This current implementation will result in a wasted lock request
issued to web site
- // that don't support it.
- // TODO: need to rewrite the managing of the m_bTransient flag, when
the resource is non yet existent
- // and the first lock on a non existed resource first creates it then
lock it.
- lock( Environment );
+ ResourceType eType = resourceTypeForLocks( Environment );
+ // when the resource is not yet present the lock is used to create it
+ // see: http://tools.ietf.org/html/rfc4918#section-7.3
+ // If the resource doesn't exists and the lock is not enabled (DAV with
+ // no lock or a simple web) the error will be dealt with inside lock()
method
+ if ( eType == NOT_FOUND ||
+ eType == DAV )
+ {
+ lock( Environment );
+ if ( eType == NOT_FOUND )
+ {
+ m_eResourceType = UNKNOWN; // lock may have created it, need
to check again
+ m_eResourceTypeForLocks = UNKNOWN;
+ }
+ }
}
- else if ( aCommand.Name == "unlock" &&
- supportsExclusiveWriteLock( Environment ) )
+ else if ( aCommand.Name == "unlock" )
{
// unlock
-
-
- unlock( Environment );
+ if ( resourceTypeForLocks( Environment ) == DAV )
+ unlock( Environment );
}
else if ( aCommand.Name == "createNewContent" &&
isFolder( Environment ) )
@@ -749,6 +757,7 @@ uno::Any SAL_CALL Content::execute(
}
SAL_INFO("ucb.ucp.webdav", "<<<<< Content::execute: end: command: " <<
aCommand.Name);
+
return aRet;
}
@@ -2385,7 +2394,15 @@ void Content::insert(
if ( bCollection )
xResAccess->MKCOL( Environment );
else
+ {
xResAccess->PUT( xInputStream, Environment );
+ }
+ // no error , set the resourcetype to unknown type
+ // the resource may have transitioned from NOT FOUND or UNKNOWN to
something else
+ // depending on the server behaviour
+ // this will force a recheck of the resource type
+ m_eResourceType = UNKNOWN;
+ m_eResourceTypeForLocks = UNKNOWN;
}
catch ( DAVException const & except )
{
@@ -2768,30 +2785,148 @@ void Content::destroy( bool bDeletePhysical )
}
}
-
-bool Content::supportsExclusiveWriteLock(
+// returns the resource type, to be checked for locks
+Content::ResourceType Content::resourceTypeForLocks(
const uno::Reference< ucb::XCommandEnvironment >& Environment )
{
- if ( getResourceType( Environment ) == DAV )
+ ResourceType eResourceTypeForLocks = UNKNOWN;
{
+ osl::MutexGuard g(m_aMutex);
+ //check if cache contains what we need, usually the first PROPFIND on
the URI has supported lock
+ std::unique_ptr< ContentProperties > xProps;
if (m_xCachedProps)
{
+ std::unique_ptr< ContentProperties > xCachedProps;
+ xCachedProps.reset( new ContentProperties(*m_xCachedProps) );
uno::Sequence< ucb::LockEntry > aSupportedLocks;
if ( m_xCachedProps->getValue( DAVProperties::SUPPORTEDLOCK )
- >>= aSupportedLocks )
+ >>= aSupportedLocks ) //get the cached value for
supportedlock
{
for ( sal_Int32 n = 0; n < aSupportedLocks.getLength(); ++n )
{
if ( aSupportedLocks[ n ].Scope
- == ucb::LockScope_EXCLUSIVE &&
+ == ucb::LockScope_EXCLUSIVE &&
aSupportedLocks[ n ].Type
- == ucb::LockType_WRITE )
- return true;
+ == ucb::LockType_WRITE )
+ eResourceTypeForLocks = DAV;
}
}
}
}
- return false;
+
+ const OUString & rURL = m_xIdentifier->getContentIdentifier();
+
+ if ( eResourceTypeForLocks == UNKNOWN )
+ {
+ // resource type for lock/unlock operations still unknown, need to ask
the server
+ std::unique_ptr< DAVResourceAccess > xResAccess;
+
+ xResAccess.reset( new DAVResourceAccess(
+ m_xContext,
+ m_rSessionFactory,
+ rURL ) );
+
+ {
+ try
+ {
+ // we need only DAV:supportedlock
+ std::vector< DAVResource > resources;
+ std::vector< OUString > aPropNames;
+ uno::Sequence< beans::Property > aProperties( 1 );
+ aProperties[ 0 ].Name = DAVProperties::SUPPORTEDLOCK;
+
+ ContentProperties::UCBNamesToDAVNames( aProperties, aPropNames
);
+ xResAccess->PROPFIND( DAVZERO, aPropNames, resources,
Environment );
+
+ // only one resource should be returned
+ if ( resources.size() == 1 )
+ {
+ // we may have received a bunch of other properties
+ // (some servers seems to do so)
+ // but we need only supported lock for this check
+ // all returned properties are in
+ // resources.properties[n].Name/.Value
+
+ std::vector< DAVPropertyValue >::iterator it;
+
+ for ( it = resources[0].properties.begin();
+ it != resources[0].properties.end(); it++)
+ {
+ if ( (*it).Name == DAVProperties::SUPPORTEDLOCK )
+ {
+ uno::Sequence< ucb::LockEntry > aSupportedLocks;
+ if ( (*it).Value >>= aSupportedLocks )
+ {
+ // this is at least a DAV, no lock confirmed
yet
+ eResourceTypeForLocks = DAV_NOLOCK;
+ for ( sal_Int32 n = 0; n <
aSupportedLocks.getLength(); ++n )
+ {
+ if ( aSupportedLocks[ n ].Scope ==
ucb::LockScope_EXCLUSIVE &&
+ aSupportedLocks[ n ].Type ==
ucb::LockType_WRITE )
+ {
+ // requested locking mode is supported
+ eResourceTypeForLocks = DAV;
+ SAL_INFO( "ucb.ucp.webdav",
"resourceTypeForLocks - URL: <"
+ <<
m_xIdentifier->getContentIdentifier() << ">, DAV lock/unlock supported");
+ break;
+ }
+ }
+ break;
+ }
+ }
+ }
+ }
+ }
+ catch ( DAVException const & e )
+ {
+ xResAccess->resetUri();
+ //grab the error code
+ switch( e.getStatus() )
+ {
+ case SC_NOT_FOUND:
+ SAL_WARN( "ucb.ucp.webdav", "resourceTypeForLocks -
URL: <"
+ << m_xIdentifier->getContentIdentifier() <<
"> was not found. ");
+ eResourceTypeForLocks = NOT_FOUND;
+ break;
+ // some servers returns this, instead
+ // TODO: probably remove it, when OPTIONS implemented
+ // the meaning of SC_FORBIDDEN is, according to
http://tools.ietf.org/html/rfc7231#section-6.5.3
+ // The 403 (Forbidden) status code indicates that the
server understood
+ // the request but refuses to authorize it
+ case SC_FORBIDDEN:
+ // this returned errors are part of base http 1.1 RFCs
+ // see:
+ case SC_NOT_IMPLEMENTED: //
http://tools.ietf.org/html/rfc7231#section-6.6.2
+ case SC_METHOD_NOT_ALLOWED: //
http://tools.ietf.org/html/rfc7231#section-6.5.5
+ // they all mean the resource is NON_DAV
+ SAL_WARN( "ucb.ucp.webdav", "resourceTypeForLocks
DAVException (SC_FORBIDDEN, SC_NOT_IMPLEMENTED or SC_METHOD_NOT_ALLOWED) - URL:
<"
+ << m_xIdentifier->getContentIdentifier() <<
">, DAV error: " << e.getError() << ", HTTP error: " << e.getStatus() );
+ eResourceTypeForLocks = NON_DAV;
+ break;
+ default:
+ //fallthrough
+ SAL_WARN( "ucb.ucp.webdav", "resourceTypeForLocks
DAVException - URL: <"
+ << m_xIdentifier->getContentIdentifier() <<
">, DAV error: " << e.getError() << ", HTTP error: " << e.getStatus() );
+ eResourceTypeForLocks = UNKNOWN;
+ }
+ }
+ }
+ }
+ osl::MutexGuard g(m_aMutex);
+ if (m_eResourceTypeForLocks == UNKNOWN)
+ {
+ m_eResourceTypeForLocks = eResourceTypeForLocks;
+ }
+ else
+ {
+ SAL_WARN_IF(
+ eResourceTypeForLocks != m_eResourceTypeForLocks, "ucb.ucp.webdav",
+ "different resource types for <" << rURL << ">: "
+ << +eResourceTypeForLocks << " vs. " << +m_eResourceTypeForLocks);
+ }
+ SAL_INFO( "ucb.ucp.webdav", "resourceTypeForLocks - URL: <"
+ << m_xIdentifier->getContentIdentifier() << ">,
m_eResourceTypeForLocks: " << m_eResourceTypeForLocks );
+ return m_eResourceTypeForLocks;
}
@@ -2849,37 +2984,56 @@ void Content::lock(
// this exception should be managed by the issuer of 'lock' command
switch(e.getError())
{
- case DAVException::DAV_LOCKED:
- {
- throw(ucb::InteractiveLockingLockedException(
- "Locked!",
- static_cast< cppu::OWeakObject * >( this ),
- task::InteractionClassification_ERROR,
- aURL,
- false ));
- }
- break;
- case DAVException::DAV_HTTP_ERROR:
- //grab the error code
- switch( e.getStatus() )
+ case DAVException::DAV_LOCKED:
{
- // this returned error is part of base http 1.1 RFCs
- case SC_NOT_IMPLEMENTED:
- case SC_METHOD_NOT_ALLOWED:
- // this is a temporary measure, means the lock method is not
supported
- // TODO: fix the behaviour of m_bTransient when the file is
first created
+ SAL_WARN( "ucb.ucp.webdav", "lock: resource already locked -
URL: <"
+ << m_xIdentifier->getContentIdentifier() << ">");
+ throw
+ ucb::InteractiveLockingLockedException(
+ "Locked!",
+ static_cast< cppu::OWeakObject * >( this ),
+ task::InteractionClassification_ERROR,
+ aURL,
+ false );
+ }
+ break;
+ case DAVException::DAV_HTTP_ERROR:
+ //grab the error code
+ switch( e.getStatus() )
+ {
+ // this returned error is part of base http 1.1 RFCs
+ case SC_NOT_IMPLEMENTED:
+ case SC_METHOD_NOT_ALLOWED:
+ SAL_WARN( "ucb.ucp.webdav", "lock: DAVException
(SC_NOT_IMPLEMENTED or SC_METHOD_NOT_ALLOWED) - URL: <"
+ << m_xIdentifier->getContentIdentifier() <<
">, DAV error: " << e.getError() << ", HTTP error: " << e.getStatus() );
+ // act as nothing happened
+ // that's because when a resource is first created
+ // the lock is sent before the put, so the resource
+ // is actually created by LOCK, locking it before
+ // doing the first PUT, but if LOCK is not supported
+ // (simple web or DAV with lock disabled) we end with
one of these two http
+ // errors
+ // details to LOCK on an unmapped (i.e. non existent)
resource are in:
+ // http://tools.ietf.org/html/rfc4918#section-7.3
+ return;
+ break;
+ default:
+ //fallthrough
+ ;
+ }
+ break;
+ case DAVException::DAV_LOCKED_SELF:
+ // we already hold the lock and it is in our internal lockstore
+ // just return as if the lock was successful
return;
break;
default:
//fallthrough
;
- }
- break;
- default:
- //fallthrough
- ;
}
+ SAL_WARN( "ucb.ucp.webdav","lock: DAVException - URL: <"
+ << m_xIdentifier->getContentIdentifier() << ">, DAV error: "
<< e.getError() << ", HTTP error: " << e.getStatus() );
cancelCommandExecution( e, Environment, false );
// Unreachable
}
@@ -2906,28 +3060,40 @@ void Content::unlock(
}
catch ( DAVException const & e )
{
- switch(e.getError())
+ switch( e.getError() )
{
- case DAVException::DAV_HTTP_ERROR:
- //grab the error code
- switch( e.getStatus() )
- {
- // this returned error is part of base http 1.1 RFCs
- case SC_NOT_IMPLEMENTED:
- case SC_METHOD_NOT_ALLOWED:
- // this is a temporary measure, means the lock method is not
supported
- // TODO: fix the behaviour of m_bTransient when the file is
first created
+ case DAVException::DAV_NOT_LOCKED:
+ SAL_WARN( "ucb.ucp.webdav", "unlock:
DAVException::DAV_NOT_LOCKED - URL: <"
+ << m_xIdentifier->getContentIdentifier() << ">");
+ // means that we don't own any lock on this resource
+ // intercepted here to remove a confusing indication to the
user
+ // unfortunately this happens in some WebDAV server
configuration
+ // acting as WebDAV and having lock/unlock enabled only
+ // for authorized user.
return;
break;
+ case DAVException::DAV_HTTP_ERROR:
+ //grab the error code
+ switch( e.getStatus() )
+ {
+ // this returned error is part of base http 1.1 RFCs
+ case SC_NOT_IMPLEMENTED:
+ case SC_METHOD_NOT_ALLOWED:
+ SAL_WARN( "ucb.ucp.webdav", "unlock: DAVException
(SC_NOT_IMPLEMENTED or SC_METHOD_NOT_ALLOWED) - URL: <"
+ << m_xIdentifier->getContentIdentifier() <<
">, DAV error: " << e.getError() << ", HTTP error: " << e.getStatus() );
+ return;
+ break;
+ default:
+ //fallthrough
+ ;
+ }
+ break;
default:
//fallthrough
;
- }
- break;
- default:
- //fallthrough
- ;
}
+ SAL_WARN( "ucb.ucp.webdav","unlock: DAVException - URL: <"
+ << m_xIdentifier->getContentIdentifier() << ">, DAV error: "
<< e.getError() << ", HTTP error: " << e.getStatus() );
cancelCommandExecution( e, Environment, false );
// Unreachable
}
@@ -3257,7 +3423,7 @@ Content::getBaseURI( const std::unique_ptr<
DAVResourceAccess > & rResAccess )
return rResAccess->getURL();
}
-
+// resource type is the type of the WebDAV resource
Content::ResourceType Content::getResourceType(
const uno::Reference< ucb::XCommandEnvironment >& xEnv,
const std::unique_ptr< DAVResourceAccess > & rResAccess,
diff --git a/ucb/source/ucp/webdav-curl/webdavcontent.hxx
b/ucb/source/ucp/webdav-curl/webdavcontent.hxx
index 86e1182f118d..bdfe0a116919 100644
--- a/ucb/source/ucp/webdav-curl/webdavcontent.hxx
+++ b/ucb/source/ucp/webdav-curl/webdavcontent.hxx
@@ -66,16 +66,20 @@ class Content : public ::ucbhelper::ContentImplHelper,
{
enum ResourceType
{
- UNKNOWN,
- NON_DAV,
- DAV
+ UNKNOWN, // the type of the Web resource is unknown
+ NOT_FOUND, // the Web resource does not exists
+ NON_DAV, // the Web resource exists but it's not DAV
+ DAV, // the type of the Web resource is DAV with lock/unlock
available
+ DAV_NOLOCK // the type of the Web resource is DAV with no lock/unlock
available
};
std::unique_ptr< DAVResourceAccess > m_xResAccess;
std::unique_ptr< CachableContentProperties > m_xCachedProps; // locally
cached props
OUString m_aEscapedTitle;
ResourceType m_eResourceType;
+ ResourceType m_eResourceTypeForLocks;
ContentProvider* m_pProvider; // No need for a ref, base class holds
object
+ rtl::Reference< DAVSessionFactory > m_rSessionFactory;
bool m_bTransient;
bool m_bCollection;
bool m_bDidGetOrHead;
@@ -166,7 +170,7 @@ private:
static bool shouldAccessNetworkAfterException( const DAVException & e );
- bool supportsExclusiveWriteLock(
+ ResourceType resourceTypeForLocks(
const css::uno::Reference< css::ucb::XCommandEnvironment >&
Environment );
// XPropertyContainer replacement
diff --git a/ucb/source/ucp/webdav-curl/webdavcontentcaps.cxx
b/ucb/source/ucp/webdav-curl/webdavcontentcaps.cxx
index 3feaf4ce694e..0db3cb3a7ad0 100644
--- a/ucb/source/ucp/webdav-curl/webdavcontentcaps.cxx
+++ b/ucb/source/ucp/webdav-curl/webdavcontentcaps.cxx
@@ -548,7 +548,8 @@ uno::Sequence< ucb::CommandInfo > Content::getCommands(
return aCmdInfo;
}
- bool bSupportsLocking = supportsExclusiveWriteLock( xEnv );
+ ResourceType eType = resourceTypeForLocks( xEnv );
+ bool bSupportsLocking = ( eType == NOT_FOUND || eType == DAV );
sal_Int32 nPos = aCmdInfo.getLength();
sal_Int32 nMoreCmds = ( bFolder ? 2 : 0 ) + ( bSupportsLocking ? 2 : 0 );
