Skip to content

Commit

Permalink
Remove FILEDosToUnixPathA conversion (#78995)
Browse files Browse the repository at this point in the history
* Remove FILEDosToUnixPathA conversion

All file paths passed to the coreclr PAL are processed by this function
to convert backslash characters to forward ones. This is causing problems
when users have directories with names containing backslashes. Such
problems were recently reported by people.

This change removes the `FILEDosToUnixPathA` and fixes one PAL test and
one coreclr test that was passing backslash separated paths.

* Remove checks for `\` on Unix and some conversions

* Update runtime to use platform specific directory separator

* Revert changes at few places and remove some code for Unix

* Fix Bundle_Extraction_To_Relative_Path_Succeeds host test

Exclude the `foo\\bar` case for non-Windows.
  • Loading branch information
janvorli authored Dec 2, 2022
1 parent 09613f3 commit 5df8e75
Show file tree
Hide file tree
Showing 47 changed files with 273 additions and 762 deletions.
2 changes: 1 addition & 1 deletion src/coreclr/debug/daccess/request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1334,7 +1334,7 @@ ClrDataAccess::GetMethodDescName(CLRDATA_ADDRESS methodDesc, unsigned int count,
nChars > 0 && nChars <= ARRAY_SIZE(path))
{
WCHAR* pFile = path + nChars - 1;
while ((pFile >= path) && (*pFile != W('\\')))
while ((pFile >= path) && (*pFile != DIRECTORY_SEPARATOR_CHAR_W))
{
pFile--;
}
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/ildasm/dis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1115,8 +1115,10 @@ BOOL Disassemble(IMDInternalImport *pImport, BYTE *ILHeader, void *GUICookie, md
pFile = NULL;
if(fopen_s(&pFile,szFileName,"rt") != 0)
{
char* pch = strrchr(szFileName,'\\');
char* pch = strrchr(szFileName, DIRECTORY_SEPARATOR_CHAR_A);
#ifdef HOST_WINDOWS
if(pch == NULL) pch = strrchr(szFileName,':');
#endif
pFile = NULL;
if(pch) fopen_s(&pFile,pch+1,"rt");
}
Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/ildasm/dman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,10 @@ void DumpManifestResources(void* GUICookie)
static WCHAR wzFileName[2048];

WszMultiByteToWideChar(CP_UTF8,0,g_szOutputFile,-1,wzFileName,2048);
wzName = wcsrchr(wzFileName,'\\');
wzName = wcsrchr(wzFileName,DIRECTORY_SEPARATOR_CHAR_W);
#ifdef HOST_WINDOWS
if(wzName == NULL) wzName = wcsrchr(wzFileName,':');
#endif
if (wzName == NULL) wzName = wzFileName;
else wzName++;

Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/inc/longfilepathwrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#define _WIN_PATH_APIS_WRAPPER_
class SString;

#ifdef HOST_WINDOWS

HMODULE
LoadLibraryExWrapper(
_In_ LPCWSTR lpLibFileName,
Expand Down Expand Up @@ -35,7 +37,6 @@ GetFileAttributesExWrapper(
_Out_writes_bytes_(sizeof(WIN32_FILE_ATTRIBUTE_DATA)) LPVOID lpFileInformation
);

#ifndef HOST_UNIX
BOOL
CopyFileExWrapper(
_In_ LPCWSTR lpExistingFileName,
Expand All @@ -46,7 +47,7 @@ CopyFileExWrapper(
_Inout_opt_ LPBOOL pbCancel,
_In_ DWORD dwCopyFlags
);
#endif //HOST_UNIX
#endif //HOST_WINDOWS

DWORD
SearchPathWrapper(
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/inc/nsutilpriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,6 @@ void MakeNestedTypeName( // throws on out of memory
#define ASSEMBLY_SEPARATOR_STR ", "
#define ASSEMBLY_SEPARATOR_WSTR W(", ")
#define ASSEMBLY_SEPARATOR_LEN 2
#define BACKSLASH_CHAR '\\'
#define BACKSLASH_WCHAR W('\\')
#define NESTED_SEPARATOR_CHAR '+'
#define NESTED_SEPARATOR_WCHAR W('+')
#define NESTED_SEPARATOR_STR "+"
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/inc/winwrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,17 @@

//File and Directory Functions which need special handling for LongFile Names
//Note only the functions which are currently used are defined
#ifdef HOST_WINDOWS
#define WszLoadLibrary LoadLibraryExWrapper
#define WszLoadLibraryEx LoadLibraryExWrapper
#define WszCreateFile CreateFileWrapper
#define WszGetFileAttributes GetFileAttributesWrapper
#define WszGetFileAttributesEx GetFileAttributesExWrapper
#else // HOST_WINDOWS
#define WszLoadLibrary LoadLibraryW
#define WszLoadLibraryEx LoadLibraryExW
#define WszCreateFile CreateFileW
#define WszGetFileAttributesEx GetFileAttributesExW
#endif // HOST_WINDOWS

//Can not use extended syntax
#define WszGetFullPathName GetFullPathNameW
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ DWORD getBreakOnBadCode()
/*****************************************************************************/
void debugError(const char* msg, const char* file, unsigned line)
{
const char* tail = strrchr(file, '\\');
const char* tail = strrchr(file, DIRECTORY_SEPARATOR_CHAR_A);
if (tail != nullptr)
{
tail = tail + 1;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void Compiler::verRaiseVerifyExceptionIfNeeded(INDEBUG(const char* msg) DEBUGARG
DEBUGARG(unsigned line))
{
#ifdef DEBUG
const char* tail = strrchr(file, '\\');
const char* tail = strrchr(file, DIRECTORY_SEPARATOR_CHAR_A);
if (tail)
{
file = tail + 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ public static partial class NativeLibrary

private static IntPtr LoadLibraryHelper(string libraryName, int _ /*flags*/, ref LoadLibErrorTracker errorTracker)
{
// do the Dos/Unix conversion
libraryName = libraryName.Replace('\\', '/');

IntPtr ret = Interop.Sys.LoadLibrary(libraryName);
if (ret == IntPtr.Zero)
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/pal/inc/mbusafecrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ extern errno_t _makepath_s( char* outDest, size_t inDestBufferSize, const char*
extern errno_t _wmakepath_s( WCHAR* outDest, size_t inDestBufferSize, const WCHAR* inDrive, const WCHAR* inDirectory, const WCHAR* inFilename, const WCHAR* inExtension );

extern errno_t _splitpath_s( const char* inPath, char* outDrive, size_t inDriveSize, char* outDirectory, size_t inDirectorySize, char* outFilename, size_t inFilenameSize, char* outExtension, size_t inExtensionSize );
extern errno_t _wsplitpath_s( const WCHAR* inPath, WCHAR* outDrive, size_t inDriveSize, WCHAR* outDirectory, size_t inDirectorySize, WCHAR* outFilename, size_t inFilenameSize, WCHAR* outExtension, size_t inExtensionSize );

extern int sprintf_s( char *string, size_t sizeInBytes, const char *format, ... );

Expand Down
223 changes: 2 additions & 221 deletions src/coreclr/pal/inc/rt/safecrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ void __cdecl _invalid_parameter(const WCHAR *_Message, const WCHAR *_FunctionNam
#define _tcsnset_s _wcsnset_s
#define _tcstok_s wcstok_s
#define _tmakepath_s _wmakepath_s
#define _tsplitpath_s _wsplitpath_s
#define _stprintf_s swprintf_s
#define _tscanf_s wscanf_s
#define _tsscanf_s swscanf_s
Expand Down Expand Up @@ -1370,13 +1369,13 @@ errno_t __cdecl _wmakepath_s(WCHAR *_Dst, size_t _SizeInWords, const WCHAR *_Dri
} while (*p != 0);

p = p - 1;
if (*p != L'/' && *p != L'\\')
if (*p != L'/')
{
if(++written >= _SizeInWords)
{
goto error_return;
}
*d++ = L'\\';
*d++ = L'/';
}
}

Expand Down Expand Up @@ -1432,224 +1431,6 @@ errno_t __cdecl _wmakepath_s(WCHAR *_Dst, size_t _SizeInWords, const WCHAR *_Dri
}
#endif

/* _wsplitpath_s */
_SAFECRT__EXTERN_C
errno_t __cdecl _wsplitpath_s(
const WCHAR *_Path,
WCHAR *_Drive, size_t _DriveSize,
WCHAR *_Dir, size_t _DirSize,
WCHAR *_Filename, size_t _FilenameSize,
WCHAR *_Ext, size_t _ExtSize
);

/* no C++ overload for _wsplitpath_s */

#if _SAFECRT_USE_INLINES || _SAFECRT_IMPL

_SAFECRT__INLINE
errno_t __cdecl _wsplitpath_s(
const WCHAR *_Path,
WCHAR *_Drive, size_t _DriveSize,
WCHAR *_Dir, size_t _DirSize,
WCHAR *_Filename, size_t _FilenameSize,
WCHAR *_Ext, size_t _ExtSize
)
{
const WCHAR *tmp;
const WCHAR *last_slash;
const WCHAR *dot;
int drive_set = 0;
size_t length = 0;
int bEinval = 0;

/* validation section */
_SAFECRT__VALIDATE_POINTER(_Path);
if ((_Drive == nullptr && _DriveSize != 0) || (_Drive != nullptr && _DriveSize == 0))
{
goto error_einval;
}
if ((_Dir == nullptr && _DirSize != 0) || (_Dir != nullptr && _DirSize == 0))
{
goto error_einval;
}
if ((_Filename == nullptr && _FilenameSize != 0) || (_Filename != nullptr && _FilenameSize == 0))
{
goto error_einval;
}
if ((_Ext == nullptr && _ExtSize != 0) || (_Ext != nullptr && _ExtSize == 0))
{
goto error_einval;
}

/* check if _Path begins with the longpath prefix */
if (_Path[0] == L'\\' && _Path[1] == L'\\' && _Path[2] == L'?' && _Path[3] == L'\\')
{
_Path += 4;
}

/* extract drive letter and ':', if any */
if (!drive_set)
{
size_t skip = _MAX_DRIVE - 2;
tmp = _Path;
while (skip > 0 && *tmp != 0)
{
skip--;
tmp++;
}
if (*tmp == L':')
{
if (_Drive != nullptr)
{
if (_DriveSize < _MAX_DRIVE)
{
goto error_erange;
}
wcsncpy_s(_Drive, _DriveSize, _Path, _MAX_DRIVE - 1);
}
_Path = tmp + 1;
}
else
{
if (_Drive != nullptr)
{
_SAFECRT__RESET_STRING(_Drive, _DriveSize);
}
}
}

/* extract path string, if any. _Path now points to the first character
* of the path, if any, or the filename or extension, if no path was
* specified. Scan ahead for the last occurrence, if any, of a '/' or
* '\' path separator character. If none is found, there is no path.
* We will also note the last '.' character found, if any, to aid in
* handling the extension.
*/
last_slash = nullptr;
dot = nullptr;
tmp = _Path;
for (; *tmp != 0; ++tmp)
{
{
if (*tmp == L'/' || *tmp == L'\\')
{
/* point to one beyond for later copy */
last_slash = tmp + 1;
}
else if (*tmp == L'.')
{
dot = tmp;
}
}
}

if (last_slash != nullptr)
{
/* found a path - copy up through last_slash or max characters
* allowed, whichever is smaller
*/
if (_Dir != nullptr) {
length = (size_t)(last_slash - _Path);
if (_DirSize <= length)
{
goto error_erange;
}
wcsncpy_s(_Dir, _DirSize, _Path, length);
}
_Path = last_slash;
}
else
{
/* there is no path */
if (_Dir != nullptr)
{
_SAFECRT__RESET_STRING(_Dir, _DirSize);
}
}

/* extract file name and extension, if any. Path now points to the
* first character of the file name, if any, or the extension if no
* file name was given. Dot points to the '.' beginning the extension,
* if any.
*/
if (dot != nullptr && (dot >= _Path))
{
/* found the marker for an extension - copy the file name up to the '.' */
if (_Filename)
{
length = (size_t)(dot - _Path);
if (_FilenameSize <= length)
{
goto error_erange;
}
wcsncpy_s(_Filename, _FilenameSize, _Path, length);
}
/* now we can get the extension - remember that tmp still points
* to the terminating nullptr character of path.
*/
if (_Ext)
{
length = (size_t)(tmp - dot);
if (_ExtSize <= length)
{
goto error_erange;
}
wcsncpy_s(_Ext, _ExtSize, dot, length);
}
}
else
{
/* found no extension, give empty extension and copy rest of
* string into fname.
*/
if (_Filename)
{
length = (size_t)(tmp - _Path);
if (_FilenameSize <= length)
{
goto error_erange;
}
wcsncpy_s(_Filename, _FilenameSize, _Path, length);
}
if (_Ext)
{
_SAFECRT__RESET_STRING(_Ext, _ExtSize);
}
}

return 0;

error_einval:
bEinval = 1;

error_erange:
if (_Drive != nullptr && _DriveSize > 0)
{
_SAFECRT__RESET_STRING(_Drive, _DriveSize);
}
if (_Dir != nullptr && _DirSize > 0)
{
_SAFECRT__RESET_STRING(_Dir, _DirSize);
}
if (_Filename != nullptr && _FilenameSize > 0)
{
_SAFECRT__RESET_STRING(_Filename, _FilenameSize);
}
if (_Ext != nullptr && _ExtSize > 0)
{
_SAFECRT__RESET_STRING(_Ext, _ExtSize);
}

if (bEinval)
{
_SAFECRT__RETURN_EINVAL;
}

_SAFECRT__RETURN_BUFFER_TOO_SMALL(_Strings, _StringSizes);
/* should never happen, but compiler can't tell */
return EINVAL;
}
#endif

/* vsprintf_s */
/*
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/pal/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ set(SOURCES
safecrt/wcsncpy_s.cpp
safecrt/wcstok_s.cpp
safecrt/wmakepath_s.cpp
safecrt/wsplitpath_s.cpp
safecrt/xtoa_s.cpp
safecrt/xtow_s.cpp
sharedmemory/sharedmemory.cpp
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/pal/src/cruntime/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,6 @@ PAL_fopen(const char * fileName, const char * mode)
goto done;
}

FILEDosToUnixPathA( UnixFileName );

/*I am not checking for the case where stat fails
*as fopen will handle the error more gracefully in case
*UnixFileName is invalid*/
Expand Down
Loading

0 comments on commit 5df8e75

Please sign in to comment.