autorecovery: clarified the role of a doc's URL/Location

as per agreement with MAV, XModel::getURL and XStorable::getLocation both need to return the logical document
URL, even when the document has been recoved from another location.
So, the DocFileLocation of the ModelImpl now is for internal purpose only. Consequently, ModelImpl's API has
been reworked to better differ between the logical and the "loaded-from" URL of the document.

5 files changed, 88 insertions, 73 deletions

diff --git a/dbaccess/source/core/dataaccess/ModelImpl.cxx b/dbaccess/source/core/dataaccess/ModelImpl.cxx
index 2db15e0cc..e2552ebea 100644
--- a/dbaccess/source/core/dataaccess/ModelImpl.cxx
+++ b/dbaccess/source/core/dataaccess/ModelImpl.cxx
@@ -811,37 +811,37 @@ const Reference< XNumberFormatsSupplier > & ODatabaseModelImpl::getNumberFormats
     }
     return m_xNumberFormatsSupplier;
 }
+
 // -----------------------------------------------------------------------------
-void ODatabaseModelImpl::attachResource( const ::rtl::OUString& _rURL, const Sequence< PropertyValue >& _rArgs )
+void ODatabaseModelImpl::setDocFileLocation( const ::rtl::OUString& i_rLoadedFrom )
 {
-    ::comphelper::NamedValueCollection aMediaDescriptor( _rArgs );
-    ENSURE_OR_THROW( _rURL.getLength(), "invalid URL" );
+    ENSURE_OR_THROW( i_rLoadedFrom.getLength(), "invalid URL" );
+    m_sDocFileLocation = i_rLoadedFrom;
+}
 
-    ::rtl::OUString sDocumentLocation( _rURL );
-    ::rtl::OUString sDocumentURL( _rURL );
+// -----------------------------------------------------------------------------
+void ODatabaseModelImpl::setResource( const ::rtl::OUString& i_rDocumentURL, const Sequence< PropertyValue >& _rArgs )
+{
+    ENSURE_OR_THROW( i_rDocumentURL.getLength(), "invalid URL" );
 
+#if OSL_DEBUG_LEVEL > 0
+    ::comphelper::NamedValueCollection aMediaDescriptor( _rArgs );
     if ( aMediaDescriptor.has( "SalvagedFile" ) )
     {
-        const ::rtl::OUString sSalvagedFile( aMediaDescriptor.getOrDefault( "SalvagedFile", ::rtl::OUString() ) );
-        // If SalvagedFile is an empty string, this indicates "the document is being recovered, but _rURL already
+        ::rtl::OUString sSalvagedFile( aMediaDescriptor.getOrDefault( "SalvagedFile", ::rtl::OUString() ) );
+        // If SalvagedFile is an empty string, this indicates "the document is being recovered, but i_rDocumentURL already
         // is the real document URL, not the temporary document location"
-        if ( sSalvagedFile.getLength() )
-        {
-            // otherwise, SalvagedFile is the URL of the document which we should mimic, though we're loaded from
-            // the file denoted by _rURL.
-            sDocumentLocation = _rURL;
-            sDocumentURL = sSalvagedFile;
-        }
-
-        if ( sSalvagedFile == _rURL )
-            // SalvagedFile doesn't carry any information anymore
-            aMediaDescriptor.remove( "SalvagedFile" );
+        if ( !sSalvagedFile.getLength() )
+            sSalvagedFile = i_rDocumentURL;
 
+        OSL_ENSURE( sSalvagedFile == i_rDocumentURL, "ODatabaseModelImpl::setResource: inconsistency!" );
+            // nowadays, setResource should only be called with the logical URL of the document
     }
+#endif
 
     m_aMediaDescriptor = stripLoadArguments( aMediaDescriptor );
 
-    switchToURL( sDocumentLocation, sDocumentURL );
+    impl_switchToLogicalURL( i_rDocumentURL );
 }
 
 // -----------------------------------------------------------------------------
@@ -1045,7 +1045,7 @@ Reference< XModel > ODatabaseModelImpl::createNewModel_deliverOwnership( bool _b
             // then nobody would call the doc's attachResource. So, we do it here, to ensure it's in a proper
             // state, fires all events, and so on.
             // #i105505# / 2009-10-02 / frank.schoenheit@sun.com
-            xModel->attachResource( ::rtl::OUString(), m_aMediaDescriptor.getPropertyValues() );
+            xModel->attachResource( xModel->getURL(), m_aMediaDescriptor.getPropertyValues() );
         }
 
         if ( _bInitialize )
@@ -1340,31 +1340,34 @@ Reference< XStorage > ODatabaseModelImpl::impl_switchToStorage_throw( const Refe
 }
 
 // -----------------------------------------------------------------------------
-void ODatabaseModelImpl::switchToURL( const ::rtl::OUString& _rDocumentLocation, const ::rtl::OUString& _rDocumentURL )
+void ODatabaseModelImpl::impl_switchToLogicalURL( const ::rtl::OUString& i_rDocumentURL )
 {
-    // register at the database context, or change registration
-    const bool bURLChanged = ( _rDocumentURL != m_sDocumentURL );
+    if ( i_rDocumentURL == m_sDocumentURL )
+        return;
+
     const ::rtl::OUString sOldURL( m_sDocumentURL );
-    if ( bURLChanged )
+    // update our name, if necessary
+    if  (   ( m_sName == m_sDocumentURL )   // our name is our old URL
+        ||  ( !m_sName.getLength() )        // we do not have a name, yet (i.e. are not registered at the database context)
+        )
     {
-        if  (   ( m_sName == m_sDocumentURL )   // our name is our old URL
-            ||  ( !m_sName.getLength() )        // we do not have a name, yet (i.e. are not registered at the database context)
-            )
+        INetURLObject aURL( i_rDocumentURL );
+        if ( aURL.GetProtocol() != INET_PROT_NOT_VALID )
         {
-            INetURLObject aURL( _rDocumentURL );
-            if ( aURL.GetProtocol() != INET_PROT_NOT_VALID )
-            {
-                m_sName = _rDocumentURL;
-                // TODO: our data source must broadcast the change of the Name property
-            }
+            m_sName = i_rDocumentURL;
+            // TODO: our data source must broadcast the change of the Name property
         }
     }
 
-    // remember both
-    m_sDocFileLocation = _rDocumentLocation.getLength() ? _rDocumentLocation : _rDocumentURL;
-    m_sDocumentURL = _rDocumentURL;
+    // remember URL
+    m_sDocumentURL = i_rDocumentURL;
 
-    if ( bURLChanged && m_pDBContext )
+    // update our location, if necessary
+    if  ( m_sDocFileLocation.getLength() == 0 )
+        m_sDocFileLocation = m_sDocumentURL;
+
+    // register at the database context, or change registration
+    if ( m_pDBContext )
     {
         if ( sOldURL.getLength() )
             m_pDBContext->databaseDocumentURLChange( sOldURL, m_sDocumentURL );
@@ -1404,10 +1407,13 @@ sal_Bool ODatabaseModelImpl::setCurrentMacroExecMode( sal_uInt16 nMacroMode )
 // -----------------------------------------------------------------------------
 ::rtl::OUString ODatabaseModelImpl::getDocumentLocation() const
 {
-    // don't return getURL() (or m_sDocumentURL, which is the same). In case we were recovered
-    // after a previous crash of OOo, m_sDocFileLocation points to the file which were loaded from,
-    // and this is the one we need for security checks.
-    return getDocFileLocation();
+    return getURL();
+    // formerly, we returned getDocFileLocation here, which is the location of the file from which we
+    // recovered the "real" document.
+    // However, during CWS autorecovery evolving, we clarified (with MAV/MT) the role of XModel::getURL and
+    // XStorable::getLocation. In this course, we agreed that for a macro security check, the *document URL*
+    // (not the recovery file URL) is to be used: The recovery file lies in the backup folder, and by definition,
+    // this folder is considered to be secure. So, the document URL needs to be used to decide about the security.
 }
 
 // -----------------------------------------------------------------------------
diff --git a/dbaccess/source/core/dataaccess/ModelImpl.hxx b/dbaccess/source/core/dataaccess/ModelImpl.hxx
index 654450c56..4c96fccdf 100644
--- a/dbaccess/source/core/dataaccess/ModelImpl.hxx
+++ b/dbaccess/source/core/dataaccess/ModelImpl.hxx
@@ -325,9 +325,13 @@ public:
     const ::comphelper::NamedValueCollection&
             getMediaDescriptor() const { return m_aMediaDescriptor; }
 
-    void    attachResource(
+    void    setResource(
                 const ::rtl::OUString& _rURL,
-                const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::PropertyValue >& _rArgs );
+                const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::PropertyValue >& _rArgs
+            );
+    void    setDocFileLocation(
+                const ::rtl::OUString& i_rLoadedFrom
+            );
 
     static ::comphelper::NamedValueCollection
             stripLoadArguments( const ::comphelper::NamedValueCollection& _rArguments );
@@ -482,19 +486,6 @@ public:
                 const ::com::sun::star::uno::Reference< ::com::sun::star::embed::XStorage >& _rxNewRootStorage
             );
 
-    /** switches to the given document location/URL
-
-        The document location is the URL of the file from which the document has been loaded.
-        The document URL is the "intended location" of the document. It differs from the location
-        if and only if the document was loaded as part of a document recovery process. In this case,
-        the location points to some temporary file, but the URL is the URL of the file which has been
-        just recovered. The next store operation would operate on the URL, not the location.
-    */
-    void    switchToURL(
-                const ::rtl::OUString& _rDocumentLocation,
-                const ::rtl::OUString& _rDocumentURL
-            );
-
     /** returns the macro mode imposed by an external instance, which passed it to attachResource
     */
     sal_Int16       getImposedMacroExecMode() const
@@ -530,6 +521,14 @@ private:
     void    impl_construct_nothrow();
     ::com::sun::star::uno::Reference< ::com::sun::star::embed::XStorage >
             impl_switchToStorage_throw( const ::com::sun::star::uno::Reference< ::com::sun::star::embed::XStorage >& _rxNewRootStorage );
+
+    /** switches to the given document URL, which denotes the logical URL of the document, not necessariy the
+        URL where the doc was loaded/recovered from
+    */
+    void    impl_switchToLogicalURL(
+                const ::rtl::OUString& i_rDocumentURL
+            );
+
 };
 
 /** a small base class for UNO components whose functionality depends on a ODatabaseModelImpl
diff --git a/dbaccess/source/core/dataaccess/databasedocument.cxx b/dbaccess/source/core/dataaccess/databasedocument.cxx
index c97f8f186..bf4b5a12b 100644
--- a/dbaccess/source/core/dataaccess/databasedocument.cxx
+++ b/dbaccess/source/core/dataaccess/databasedocument.cxx
@@ -662,6 +662,10 @@ void SAL_CALL ODatabaseDocument::storeToRecoveryFile( const ::rtl::OUString& i_T
 void SAL_CALL ODatabaseDocument::recoverFromFile( const ::rtl::OUString& i_SourceLocation, const ::rtl::OUString& i_SalvagedFile, const Sequence< PropertyValue >& i_MediaDescriptor ) throw ( RuntimeException, IOException, WrappedTargetException )
 {
     DocumentGuard aGuard( *this, DocumentGuard::InitMethod );
+
+    if ( i_SourceLocation.getLength() == 0 )
+        throw IllegalArgumentException( ::rtl::OUString(), *this, 1 );
+
     try
     {
         // load the document itself, by simply delegating to our "load" method
@@ -678,9 +682,12 @@ void SAL_CALL ODatabaseDocument::recoverFromFile( const ::rtl::OUString& i_Sourc
         // So, everything else is done when the first controller is connected.
         m_bHasBeenRecovered = true;
 
+        // tell the impl that we've been loaded from the given location
+        m_pImpl->setDocFileLocation( i_SourceLocation );
+
         // by definition (of XDocumentRecovery), we're responsible for delivering a fully-initialized document,
         // which includes an attachResource call.
-        impl_attachResource( i_SourceLocation, aMediaDescriptor.getPropertyValues(), aGuard );
+        impl_attachResource( i_SalvagedFile, aMediaDescriptor.getPropertyValues(), aGuard );
         // <- SYNCHRONIZED
     }
     catch( const Exception& )
@@ -708,10 +715,10 @@ sal_Bool SAL_CALL ODatabaseDocument::attachResource( const ::rtl::OUString& _rUR
 }
 
 // -----------------------------------------------------------------------------
-sal_Bool ODatabaseDocument::impl_attachResource( const ::rtl::OUString& i_rURL, const Sequence< PropertyValue >& i_rMediaDescriptor,
-            DocumentGuard& _rDocGuard )
+sal_Bool ODatabaseDocument::impl_attachResource( const ::rtl::OUString& i_rLogicalDocumentURL,
+            const Sequence< PropertyValue >& i_rMediaDescriptor, DocumentGuard& _rDocGuard )
 {
-    if  (   ( i_rURL == getURL() )
+    if  (   ( i_rLogicalDocumentURL == getURL() )
         &&  ( i_rMediaDescriptor.getLength() == 1 )
         &&  ( i_rMediaDescriptor[0].Name.compareToAscii( "BreakMacroSignature" ) == 0 )
         )
@@ -722,14 +729,14 @@ sal_Bool ODatabaseDocument::impl_attachResource( const ::rtl::OUString& i_rURL,
             // (we do not support macro signatures, so we can ignore this call)
     }
 
-    // if no URL has been provided, the caller was lazy enough to not call our getLocation/getURL - which is allowed ...
-    ::rtl::OUString sURL( i_rURL );
-    if ( !sURL.getLength() )
-        sURL = getLocation();
-    if ( !sURL.getLength() )
-        sURL = getURL();
+    // if no URL has been provided, the caller was lazy enough to not call our getURL - which is not allowed anymore,
+    // now since getURL and getLocation both return the same, so calling one of those should be simple.
+    ::rtl::OUString sDocumentURL( i_rLogicalDocumentURL );
+    OSL_ENSURE( sDocumentURL.getLength(), "ODatabaseDocument::impl_attachResource: invalid URL!" );
+    if ( !sDocumentURL.getLength() )
+        sDocumentURL = getURL();
 
-    m_pImpl->attachResource( sURL, i_rMediaDescriptor );
+    m_pImpl->setResource( sDocumentURL, i_rMediaDescriptor );
 
     if ( impl_isInitializing() )
     {   // this means we've just been loaded, and this is the attachResource call which follows
@@ -918,7 +925,9 @@ sal_Bool SAL_CALL ODatabaseDocument::hasLocation(  ) throw (RuntimeException)
 ::rtl::OUString SAL_CALL ODatabaseDocument::getLocation(  ) throw (RuntimeException)
 {
     DocumentGuard aGuard( *this, DocumentGuard::MethodWithoutInit );
-    return m_pImpl->getDocFileLocation();
+    return m_pImpl->getURL();
+        // both XStorable::getLocation and XModel::getURL have to return the URL of the document, *not*
+        // the location of the file which the docunment was possibly recovered from (which would be getDocFileLocation)
 }
 // -----------------------------------------------------------------------------
 sal_Bool SAL_CALL ODatabaseDocument::isReadonly(  ) throw (RuntimeException)
@@ -1040,7 +1049,8 @@ void ODatabaseDocument::impl_storeAs_throw( const ::rtl::OUString& _rURL, const
         impl_storeToStorage_throw( xCurrentStorage, aMediaDescriptor, _rGuard );
 
         // success - tell our impl
-        m_pImpl->attachResource( _rURL, aMediaDescriptor );
+        m_pImpl->setDocFileLocation( _rURL );
+        m_pImpl->setResource( _rURL, aMediaDescriptor );
 
         // if we are in an initialization process, then this is finished, now that we stored the document
         if ( bIsInitializationProcess )
diff --git a/dbaccess/source/core/dataaccess/databasedocument.hxx b/dbaccess/source/core/dataaccess/databasedocument.hxx
index 7972d30e9..93a992817 100644
--- a/dbaccess/source/core/dataaccess/databasedocument.hxx
+++ b/dbaccess/source/core/dataaccess/databasedocument.hxx
@@ -613,8 +613,8 @@ private:
 
     /** impl-version of attachResource
 
-        @param  i_rURL
-            denotes the URL the document was loaded from
+        @param  i_rLogicalDocumentURL
+            denotes the logical URL of the document, to be reported by getURL/getLocation
         @param  i_rMediaDescriptor
             denotes additional document parameters
         @param  _rDocGuard
@@ -622,7 +622,7 @@ private:
         
     */
     sal_Bool    impl_attachResource(
-                    const ::rtl::OUString& i_rURL,
+                    const ::rtl::OUString& i_rLogicalDocumentURL,
                     const ::com::sun::star::uno::Sequence< ::com::sun::star::beans::PropertyValue >& i_rMediaDescriptor,
                     DocumentGuard& _rDocGuard
                 );
diff --git a/dbaccess/source/ui/browser/dataview.cxx b/dbaccess/source/ui/browser/dataview.cxx
index 5450b55cf..733e814b5 100644
--- a/dbaccess/source/ui/browser/dataview.cxx
+++ b/dbaccess/source/ui/browser/dataview.cxx
@@ -236,7 +236,7 @@ namespace dbaui
                 {
                     ::comphelper::NamedValueCollection aArgs( xModel->getArgs() );
                     aArgs.remove( "Hidden" );
-                    xModel->attachResource( ::rtl::OUString(), aArgs.getPropertyValues() );
+                    xModel->attachResource( xModel->getURL(), aArgs.getPropertyValues() );
                 }
             }
             catch( const Exception& )