tdf#60237 - correct the behaviour of the Overwrite property

If the Overwrite property of the MediaDescriptor is set to false, try to
write the file before trying to rename or copy it. If the file already
exists, raise an error (ERRCODE_IO_ALREADYEXISTS). In addition, the
documentation about the default option was corrected.

Change-Id: I1031dca3a039343fc599d194fcaa99a20dd13e2a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110089
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

3 files changed, 63 insertions, 22 deletions

diff --git a/offapi/com/sun/star/document/MediaDescriptor.idl b/offapi/com/sun/star/document/MediaDescriptor.idl
index 119ed0e4c3fb..73ea2cacba93 100644
--- a/offapi/com/sun/star/document/MediaDescriptor.idl
+++ b/offapi/com/sun/star/document/MediaDescriptor.idl
@@ -320,8 +320,8 @@ service MediaDescriptor
     /** overwrite any existing file
 
         <p>
-        For storing only: overwrite any existing file, default is `FALSE`,
-        so an error occurs if the target file already exists.
+        For storing only: overwrite any existing file, default is `TRUE`.
+        Setting this to `FALSE` raises an error, if the target file already exists.
         </p>
      */
     [optional,property] boolean Overwrite;
diff --git a/sfx2/qa/cppunit/test_misc.cxx b/sfx2/qa/cppunit/test_misc.cxx
index f6039b0e68d8..02c2ed356f88 100644
--- a/sfx2/qa/cppunit/test_misc.cxx
+++ b/sfx2/qa/cppunit/test_misc.cxx
@@ -25,11 +25,13 @@
 #include <com/sun/star/packages/zip/ZipFileAccess.hpp>
 #include <com/sun/star/frame/Desktop.hpp>
 #include <com/sun/star/frame/XStorable.hpp>
+#include <com/sun/star/ucb/NameClashException.hpp>
 
 #include <test/bootstrapfixture.hxx>
 #include <test/xmltesttools.hxx>
 #include <unotest/macros_test.hxx>
 
+#include <unotools/mediadescriptor.hxx>
 #include <unotools/ucbstreamhelper.hxx>
 #include <comphelper/propertysequence.hxx>
 #include <comphelper/processfactory.hxx>
@@ -192,6 +194,37 @@ CPPUNIT_TEST_FIXTURE(MiscTest, testHardLinks)
 #endif
 }
 
+CPPUNIT_TEST_FIXTURE(MiscTest, testOverwrite)
+{
+    // tdf#60237 - try to overwrite an existing file using the different settings of the Overwrite option
+    utl::TempFile aTempFile;
+    aTempFile.EnableKillingFile();
+    uno::Reference<lang::XComponent> xComponent
+        = loadFromDesktop(aTempFile.GetURL(), "com.sun.star.text.TextDocument");
+    CPPUNIT_ASSERT(xComponent.is());
+    uno::Reference<frame::XStorable> xStorable(xComponent, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xStorable.is());
+
+    // overwrite the file using the default case of the Overwrite option (true)
+    CPPUNIT_ASSERT_NO_THROW(xStorable->storeToURL(aTempFile.GetURL(), {}));
+
+    // explicitly overwrite the file using the Overwrite option
+    CPPUNIT_ASSERT_NO_THROW(xStorable->storeToURL(
+        aTempFile.GetURL(),
+        comphelper::InitPropertySequence({ { "Overwrite", uno::makeAny(true) } })));
+
+    try
+    {
+        // overwrite an existing file with the Overwrite flag set to false
+        xStorable->storeToURL(aTempFile.GetURL(), comphelper::InitPropertySequence(
+                                                      { { "Overwrite", uno::makeAny(false) } }));
+        CPPUNIT_ASSERT_MESSAGE("We expect an exception on overwriting an existing file", false);
+    }
+    catch (const css::uno::Exception&)
+    {
+    }
+}
+
 }
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sfx2/source/doc/docfile.cxx b/sfx2/source/doc/docfile.cxx
index 66e95d348b24..767a4b8ce6a7 100644
--- a/sfx2/source/doc/docfile.cxx
+++ b/sfx2/source/doc/docfile.cxx
@@ -51,6 +51,7 @@
 #include <com/sun/star/ucb/InteractiveLockingLockedException.hpp>
 #include <com/sun/star/ucb/InteractiveNetworkWriteException.hpp>
 #include <com/sun/star/ucb/Lock.hpp>
+#include <com/sun/star/ucb/NameClashException.hpp>
 #include <com/sun/star/ucb/XCommandEnvironment.hpp>
 #include <com/sun/star/ucb/XProgressHandler.hpp>
 #include <com/sun/star/io/XOutputStream.hpp>
@@ -1927,22 +1928,30 @@ void SfxMedium::TransactedTransferForFS_Impl( const INetURLObject& aSource,
 
         try
         {
-            OUString aSourceMainURL = aSource.GetMainURL(INetURLObject::DecodeMechanism::NONE);
-            OUString aDestMainURL = aDest.GetMainURL(INetURLObject::DecodeMechanism::NONE);
-
-            sal_uInt64 nAttributes = GetDefaultFileAttributes(aDestMainURL);
-            if (IsFileMovable(aDest)
-                && osl::File::replace(aSourceMainURL, aDestMainURL) == osl::FileBase::E_None)
+            // tdf#60237 - if the OverWrite property of the MediaDescriptor is set to false,
+            // try to write the file before trying to rename or copy it
+            if (!(bOverWrite
+                  && ::utl::UCBContentHelper::IsDocument(
+                      aDest.GetMainURL(INetURLObject::DecodeMechanism::NONE))))
             {
-                if (nAttributes)
-                    // Adjust attributes, source might be created with
-                    // the osl_File_OpenFlag_Private flag.
-                    osl::File::setAttributes(aDestMainURL, nAttributes);
+                Reference< XInputStream > aTempInput = aTempCont.openStream();
+                aOriginalContent.writeStream( aTempInput, bOverWrite );
                 bResult = true;
-            }
-            else
-            {
-                if (bOverWrite && ::utl::UCBContentHelper::IsDocument(aDestMainURL))
+            } else {
+                OUString aSourceMainURL = aSource.GetMainURL(INetURLObject::DecodeMechanism::NONE);
+                OUString aDestMainURL = aDest.GetMainURL(INetURLObject::DecodeMechanism::NONE);
+
+                sal_uInt64 nAttributes = GetDefaultFileAttributes(aDestMainURL);
+                if (IsFileMovable(aDest)
+                    && osl::File::replace(aSourceMainURL, aDestMainURL) == osl::FileBase::E_None)
+                {
+                    if (nAttributes)
+                        // Adjust attributes, source might be created with
+                        // the osl_File_OpenFlag_Private flag.
+                        osl::File::setAttributes(aDestMainURL, nAttributes);
+                    bResult = true;
+                }
+                else
                 {
                     if( pImpl->m_aBackupURL.isEmpty() )
                         DoInternalBackup_Impl( aOriginalContent );
@@ -1960,12 +1969,6 @@ void SfxMedium::TransactedTransferForFS_Impl( const INetURLObject& aSource,
                         pImpl->m_eError = ERRCODE_SFX_CANTCREATEBACKUP;
                     }
                 }
-                else
-                {
-                    Reference< XInputStream > aTempInput = aTempCont.openStream();
-                    aOriginalContent.writeStream( aTempInput, bOverWrite );
-                    bResult = true;
-                }
             }
         }
         catch ( const css::ucb::CommandAbortedException& )
@@ -1987,6 +1990,11 @@ void SfxMedium::TransactedTransferForFS_Impl( const INetURLObject& aSource,
             else
                 pImpl->m_eError = ERRCODE_IO_GENERAL;
         }
+        // tdf#60237 - if the file is already present, raise the appropriate error
+        catch (const css::ucb::NameClashException& )
+        {
+            pImpl->m_eError = ERRCODE_IO_ALREADYEXISTS;
+        }
         catch ( const css::uno::Exception& )
         {
             pImpl->m_eError = ERRCODE_IO_GENERAL;