Don't change process current directory in osl_getAbsoluteFileURL

This removes the need to synchronize using global mutex, and allows
to get correct result when base directory is unavailable.

Change-Id: I9ae70a7d0e8f0840a533a2d0d222336cc0bf0b4e
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/110163
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

4 files changed, 76 insertions, 58 deletions

diff --git a/sal/osl/w32/dllentry.cxx b/sal/osl/w32/dllentry.cxx
index 9493e59bc982..06857df6bbc5 100644
--- a/sal/osl/w32/dllentry.cxx
+++ b/sal/osl/w32/dllentry.cxx
@@ -88,9 +88,6 @@ static BOOL WINAPI RawDllMain( HINSTANCE, DWORD fdwReason, LPVOID )
                 /* initialize global mutex */
                 g_Mutex = osl_createMutex();
 
-                /* initialize "current directory" mutex */
-                g_CurrentDirectoryMutex = osl_createMutex();
-
                 g_dwTLSTextEncodingIndex = TlsAlloc();
                 InitializeCriticalSection( &g_ThreadKeyListCS );
 
@@ -108,8 +105,6 @@ static BOOL WINAPI RawDllMain( HINSTANCE, DWORD fdwReason, LPVOID )
 
             osl_destroyMutex( g_Mutex );
 
-            osl_destroyMutex( g_CurrentDirectoryMutex );
-
             /*
 
             On a product build memory management finalization might
diff --git a/sal/osl/w32/file_url.cxx b/sal/osl/w32/file_url.cxx
index a0cb52257b96..e71da15a66b9 100644
--- a/sal/osl/w32/file_url.cxx
+++ b/sal/osl/w32/file_url.cxx
@@ -28,6 +28,7 @@
 #include "file_error.hxx"
 
 #include <rtl/alloc.h>
+#include <rtl/character.hxx>
 #include <rtl/ustring.hxx>
 #include <osl/mutex.h>
 #include <o3tl/char16_t2wchar_t.hxx>
@@ -40,8 +41,6 @@
 
 // FileURL functions
 
-oslMutex g_CurrentDirectoryMutex = nullptr; /* Initialized in dllentry.c */
-
 static bool IsValidFilePathComponent(
     sal_Unicode const * lpComponent, sal_Unicode const **lppComponentEnd,
     DWORD dwFlags)
@@ -991,9 +990,40 @@ oslFileError SAL_CALL osl_searchFileURL(
     return error;
 }
 
+namespace
+{
+bool startsWithDriveColon(const rtl_uString* path)
+{
+    return rtl::isAsciiAlpha(path->buffer[0]) && path->buffer[1] == ':';
+}
+bool startsWithDriveColonSlash(const rtl_uString* path)
+{
+    return startsWithDriveColon(path) && path->buffer[2] == '\\';
+}
+// An absolute path starts either with \\ (an UNC or device path like \\.\ or \\?\)
+// or with a ASCII alpha character followed by colon followed by backslash.
+bool isAbsolute(const rtl_uString* path)
+{
+    return (path->buffer[0] == '\\' && path->buffer[1] == '\\') || startsWithDriveColonSlash(path);
+}
+bool onSameDrive(const rtl_uString* path1, const rtl_uString* path2)
+{
+    return rtl::toAsciiUpperCase(path1->buffer[0]) == rtl::toAsciiUpperCase(path2->buffer[0])
+           && rtl::toAsciiUpperCase(path1->buffer[1]) == rtl::toAsciiUpperCase(path2->buffer[1]);
+}
+OUString combinePath(rtl_uString* basePath, const sal_Unicode* relPath)
+{
+    const bool needSep = basePath->buffer[basePath->length - 1] != '\\' && relPath[0] != '\\';
+    const auto sSeparator = needSep ? std::u16string_view(u"\\") : std::u16string_view();
+    if (basePath->buffer[basePath->length - 1] == '\\' && relPath[0] == '\\')
+        ++relPath; // avoid two agjacent backslashes
+    return OUString::unacquired(&basePath) + sSeparator + relPath;
+}
+}
+
 oslFileError SAL_CALL osl_getAbsoluteFileURL( rtl_uString* ustrBaseURL, rtl_uString* ustrRelativeURL, rtl_uString** pustrAbsoluteURL )
 {
-    oslFileError    eError;
+    oslFileError eError = osl_File_E_None;
     rtl_uString     *ustrRelSysPath = nullptr;
     rtl_uString     *ustrBaseSysPath = nullptr;
 
@@ -1001,64 +1031,63 @@ oslFileError SAL_CALL osl_getAbsoluteFileURL( rtl_uString* ustrBaseURL, rtl_uStr
     {
         eError = osl_getSystemPathFromFileURL_( ustrBaseURL, &ustrBaseSysPath, false );
         OSL_ENSURE( osl_File_E_None == eError, "osl_getAbsoluteFileURL called with relative or invalid base URL" );
-
-        eError = osl_getSystemPathFromFileURL_( ustrRelativeURL, &ustrRelSysPath, true );
     }
-    else
+    if (eError == osl_File_E_None)
     {
-        eError = osl_getSystemPathFromFileURL_( ustrRelativeURL, &ustrRelSysPath, false );
+        eError = osl_getSystemPathFromFileURL_(ustrRelativeURL, &ustrRelSysPath,
+                                               ustrBaseSysPath != nullptr);
         OSL_ENSURE( osl_File_E_None == eError, "osl_getAbsoluteFileURL called with empty base URL and/or invalid relative URL" );
     }
 
     if ( !eError )
     {
-        ::osl::LongPathBuffer< sal_Unicode > aBuffer( MAX_LONG_PATH );
-        ::osl::LongPathBuffer< sal_Unicode > aCurrentDir( MAX_LONG_PATH );
-        LPWSTR  lpFilePart = nullptr;
-        DWORD   dwResult;
-
-/*@@@ToDo
-  Bad, bad hack, this only works if the base path
-  really exists which is not necessary according
-  to RFC2396
-  The whole FileURL implementation should be merged
-  with the rtl/uri class.
-*/
-        if ( ustrBaseSysPath )
-        {
-            osl_acquireMutex( g_CurrentDirectoryMutex );
-
-            GetCurrentDirectoryW( aCurrentDir.getBufSizeInSymbols(), o3tl::toW(aCurrentDir) );
-            SetCurrentDirectoryW( o3tl::toW(ustrBaseSysPath->buffer) );
-        }
-
-        dwResult = GetFullPathNameW( o3tl::toW(ustrRelSysPath->buffer), aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer), &lpFilePart );
+        OUString sResultPath;
 
-        if ( ustrBaseSysPath )
+        // If ustrRelSysPath is absolute, we don't need ustrBaseSysPath.
+        if (ustrBaseSysPath && !isAbsolute(ustrRelSysPath))
         {
-            SetCurrentDirectoryW( o3tl::toW(aCurrentDir) );
+            // ustrBaseSysPath is known here to be a valid absolute path -> its first two characters
+            // are ASCII (either alpha + colon, or double backslashes)
 
-            osl_releaseMutex( g_CurrentDirectoryMutex );
-        }
+            // Don't use SetCurrentDirectoryW together with GetFullPathNameW, because:
+            // (a) it needs synchronization and may affect threads that may access relative paths;
+            // (b) it would fail or give wrong results for non-existing base path (possible in URL).
 
-        if ( dwResult )
-        {
-            if ( dwResult >= aBuffer.getBufSizeInSymbols() )
-                eError = osl_File_E_INVAL;
-            else
+            if (startsWithDriveColon(ustrRelSysPath))
             {
-                rtl_uString *ustrAbsSysPath = nullptr;
-
-                rtl_uString_newFromStr( &ustrAbsSysPath, aBuffer );
-
-                eError = osl_getFileURLFromSystemPath( ustrAbsSysPath, pustrAbsoluteURL );
+                // Special case: a path relative to a specific drive's current directory.
+                // Should we error out here?
 
-                if ( ustrAbsSysPath )
-                    rtl_uString_release( ustrAbsSysPath );
+                if (onSameDrive(ustrRelSysPath, ustrBaseSysPath))
+                {
+                    // If ustrBaseSysPath defines a path on the same drive, combine them
+                    sResultPath = combinePath(ustrBaseSysPath, ustrRelSysPath->buffer + 2);
+                }
+                else
+                {
+                    // Otherwise just call GetFullPathNameW to get current directory on that drive
+                    osl::LongPathBuffer<wchar_t> aBuf(MAX_LONG_PATH);
+                    DWORD dwResult = GetFullPathNameW(o3tl::toW(ustrRelSysPath->buffer),
+                                                      aBuf.getBufSizeInSymbols(), aBuf, nullptr);
+                    if (dwResult)
+                    {
+                        if (dwResult >= aBuf.getBufSizeInSymbols())
+                            eError = osl_File_E_INVAL;
+                        else
+                            sResultPath = o3tl::toU(aBuf);
+                    }
+                    else
+                        eError = oslTranslateFileError(GetLastError());
+                }
             }
+            else
+                sResultPath = combinePath(ustrBaseSysPath, ustrRelSysPath->buffer);
         }
         else
-            eError = oslTranslateFileError( GetLastError() );
+            sResultPath = OUString::unacquired(&ustrRelSysPath);
+
+        if (eError == osl_File_E_None)
+            eError = osl_getFileURLFromSystemPath(sResultPath.pData, pustrAbsoluteURL);
     }
 
     if ( ustrBaseSysPath )
diff --git a/sal/osl/w32/file_url.hxx b/sal/osl/w32/file_url.hxx
index 37bf86ca3b12..22ad1b89af81 100644
--- a/sal/osl/w32/file_url.hxx
+++ b/sal/osl/w32/file_url.hxx
@@ -23,7 +23,7 @@
 #include <sal/types.h>
 #include <rtl/ustring.h>
 #include <osl/file.h>
-#include <osl/mutex.h>
+#include <osl/mutex.hxx>
 
 #if !defined WIN32_LEAN_AND_MEAN
 # define WIN32_LEAN_AND_MEAN
@@ -65,8 +65,6 @@ oslFileError osl_getSystemPathFromFileURL_ (
     bool bAllowRelative
 );
 
-extern oslMutex g_CurrentDirectoryMutex;
-
 #endif
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sal/osl/w32/process.cxx b/sal/osl/w32/process.cxx
index 205d6415e035..065415f2cf86 100644
--- a/sal/osl/w32/process.cxx
+++ b/sal/osl/w32/process.cxx
@@ -471,11 +471,7 @@ oslProcessError SAL_CALL osl_clearEnvironment(rtl_uString *ustrVar)
 oslProcessError SAL_CALL osl_getProcessWorkingDir( rtl_uString **pustrWorkingDir )
 {
     ::osl::LongPathBuffer< sal_Unicode > aBuffer( MAX_LONG_PATH );
-    DWORD   dwLen = 0;
-
-    osl_acquireMutex( g_CurrentDirectoryMutex );
-    dwLen = GetCurrentDirectoryW( aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer) );
-    osl_releaseMutex( g_CurrentDirectoryMutex );
+    DWORD dwLen = GetCurrentDirectoryW(aBuffer.getBufSizeInSymbols(), o3tl::toW(aBuffer));
 
     if ( dwLen && dwLen < aBuffer.getBufSizeInSymbols() )
     {