From a60098da397d0a4aa39e213866bc5d1c41d4503c Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Thu, 2 Feb 2017 10:16:34 -0800 Subject: [PATCH] Rewrite portions of EbrFile handling to use UTF16 (#1891) * Rewrite path mapper to simplify code and support UTF16 path names. * Rewrite Ebr file apis to use new path mapper. * Remove dead code. * fixed unit tests to build after refactor. * Add NSFileManager test to use special characters. * EbrGet/SetWritablePath are now IwGet/SetWritablePath, and support UTF-16. Fixes #1875 --- Frameworks/Foundation/NSPathUtilities.mm | 4 +- Frameworks/Starboard/AssetFile.cpp | 292 +++--------------- Frameworks/Starboard/EbrFile.cpp | 94 ++---- Frameworks/Starboard/EbrIOFile.cpp | 3 +- Frameworks/Starboard/PathMapper.cpp | 148 +++++++++ Frameworks/Starboard/PathMapper.h | 50 +-- .../StarboardXaml/ApplicationCompositor.cpp | 7 +- Frameworks/include/Platform/EbrPlatform.h | 9 +- build/Starboard/dll/Starboard.def | 8 +- build/Starboard/lib/StarboardLib.vcxproj | 3 +- .../Starboard/Starboard.UnitTests.vcxproj | 9 +- .../Foundation/NSFileManagerTests.mm | 13 +- tests/unittests/Starboard/PathMapperTests.mm | 103 ++++++ 13 files changed, 377 insertions(+), 366 deletions(-) create mode 100644 Frameworks/Starboard/PathMapper.cpp create mode 100644 tests/unittests/Starboard/PathMapperTests.mm diff --git a/Frameworks/Foundation/NSPathUtilities.mm b/Frameworks/Foundation/NSPathUtilities.mm index 63617f514d..0349eea3b7 100644 --- a/Frameworks/Foundation/NSPathUtilities.mm +++ b/Frameworks/Foundation/NSPathUtilities.mm @@ -39,7 +39,7 @@ // Helper that gets the path for a folder dirName under the current app's AppData/Local... directory, // creating the folder if necessary NSString* _getCreateAppDataLocalDir(const char* dirName) { - auto ret = [NSString stringWithFormat:@"%hs/%hs", EbrGetWritableFolder(), dirName]; + auto ret = [NSString stringWithFormat:@"%S/%hs", IwGetWritableFolder(), dirName]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } @@ -47,7 +47,7 @@ // Override for when a higher-level directory needs to be created first (eg: Foo1/Foo2/) NSString* _getCreateAppDataLocalDir(const char* dirName1, const char* dirName2) { _getCreateAppDataLocalDir(dirName1); - auto ret = [NSString stringWithFormat:@"%hs/%hs/%hs", EbrGetWritableFolder(), dirName1, dirName2]; + auto ret = [NSString stringWithFormat:@"%S/%hs/%hs", IwGetWritableFolder(), dirName1, dirName2]; _mkdir([ret cStringUsingEncoding:NSUTF8StringEncoding]); return ret; } diff --git a/Frameworks/Starboard/AssetFile.cpp b/Frameworks/Starboard/AssetFile.cpp index 13c7659fa7..5c05f730e2 100644 --- a/Frameworks/Starboard/AssetFile.cpp +++ b/Frameworks/Starboard/AssetFile.cpp @@ -14,228 +14,39 @@ // //****************************************************************************** -#include -#include -#include -#include +#include "Starboard.h" +#include "ErrorHandling.h" +#include "StringHelpers.h" #include -#include -#include -#include "Starboard.h" -#include "Platform/EbrPlatform.h" #include "AssetFile.h" #include "PathMapper.h" -#include "LoggingNative.h" static const wchar_t* TAG = L"AssetFile"; -#define strtok_r strtok_s - -char CPathMapper::currentDir[4096]; - -void appendPath(char* curpath, const char* path) { - char copy[4096]; - - strcpy_s(copy, path); - - char* save; - char* curToken = strtok_r(copy, "/\\", &save); - char* curpathEnd = curpath + strlen(curpath); - while (curToken) { - if (strlen(curToken) == 0) { - curToken = strtok_r(NULL, "/\\", &save); - } - int tokenLen = strlen(curToken); - if (strcmp(curToken, "~") == 0) { - strcpy_s(curpath, 2048, "/home"); - curpathEnd = curpath + strlen(curpath); - } else { - strcat_s(curpathEnd, 2048, "/"); - curpathEnd++; - strcat_s(curpathEnd, 2048, curToken); - curpathEnd += tokenLen; - } - curToken = strtok_r(NULL, "/\\", &save); - } - if (strcmp(curpath, "") == 0) - strcpy_s(curpath, 2048, "/"); -} - -static void EscapePath(char* dest, const char* src) { - while (*dest) - dest++; - - while (*src) { - switch (*src) { - case '?': - *dest = '+'; - break; - - default: - *dest = *src; - } - - dest++; - src++; - } - - *dest = 0; -} - -bool convertPath(char* filePath, const char* relativePath) { - if (!EbrGetRootMapping(NULL, filePath, 4096)) - return false; - int curComponent = 0; - - char copy[4096]; - - strcpy_s(copy, relativePath); - - char* save; - char* curToken = strtok_r(copy, "/", &save); - char* filePathEnd = filePath + strlen(filePath); - while (curToken) { - if (strlen(curToken) == 0 || strcmp(curToken, ".") == 0) { - curToken = strtok_r(NULL, "/", &save); - continue; - } - - if (curComponent == 0) { - if (!EbrGetRootMapping(curToken, filePath, 4096)) - return false; - filePathEnd = filePath + strlen(filePath); - } else { - strcat_s(filePathEnd, 2048, "\\"); - filePathEnd++; - EscapePath(filePathEnd, curToken); - filePathEnd += strlen(curToken); - } - curComponent++; - - curToken = strtok_r(NULL, "/\\", &save); - } - - if (strcmp(filePath, "") == 0) - return false; - - return true; -} - -bool fixPath(char* outPath, const char* relativePath) { - strcpy_s(outPath, 2048, ""); - int curComponent = 0; - - if (relativePath[0] != '/') - return false; - - char copy[4096]; - - strcpy_s(copy, relativePath); - - char* save; - char* curToken = strtok_r(copy, "/", &save); - char* outPathEnd = outPath + strlen(outPath); - while (curToken) { - if (strlen(curToken) == 0 || strcmp(curToken, ".") == 0) { - curToken = strtok_r(NULL, "/", &save); - continue; - } - if (strcmp(curToken, "..") == 0) { - if (curComponent == 0) { - return false; - } - // Move back one directory - char* curPos = outPath + strlen(outPath); - while (curPos >= outPath) { - if (*curPos == '/') { - *curPos = 0; - break; - } - curPos--; - } - // Impossible! - if (curPos < outPath) { - return false; - } - if (curPos == outPath) { - strcpy_s(outPath, 2048, "/"); - } - curComponent--; - curToken = strtok_r(NULL, "/", &save); - outPathEnd = outPath + strlen(outPath); - continue; - } - - strcat_s(outPathEnd, 2048, "/"); - outPathEnd++; - strcat_s(outPathEnd, 2048, curToken); - outPathEnd += strlen(curToken); - curComponent++; - - curToken = strtok_r(NULL, "/", &save); - } - if (strcmp(outPath, "") == 0) { - strcpy_s(outPath, 2048, "/"); - } - - return true; -} - -char* CPathMapper::FixedPath() { - if (fixedValid) - return fixedPath; - return NULL; -} - -char* CPathMapper::MappedPath() { - if (fixedValid && mappedValid) - return mappedPath; - return NULL; -} - -CPathMapper::CPathMapper(const char* path) { - char relativePath[4096]; - strcpy_s(relativePath, ""); - - if (path[0] != '/') { - appendPath(relativePath, currentDir); - } - appendPath(relativePath, path); - fixedValid = fixPath(fixedPath, relativePath); - mappedValid = convertPath(mappedPath, fixedPath); -} - -void ScanAssets() { -} - struct EbrDir { EbrDirReader* curReader; }; class EbrFSDirReader : public EbrDirReader { public: - char path[4096]; - char startPath[4096]; + std::wstring path; + std::wstring startPath; HANDLE findHandle; WIN32_FIND_DATAW data; bool isFirst; - static EbrDirReader* open(const char* path); + static EbrDirReader* open(const std::wstring& path); virtual ~EbrFSDirReader(); virtual bool readNext(EbrDir* curDir, EbrDirEnt* end); }; -EbrDirReader* EbrFSDirReader::open(const char* path) { - CPathMapper map(path); - if (!map.MappedPath()) - return NULL; - +EbrDirReader* EbrFSDirReader::open(const std::wstring& path) { EbrFSDirReader* ret = new EbrFSDirReader(); - sprintf_s(ret->path, "%s\\*", (const char*)CPathMapper(path)); - strcpy_s(ret->startPath, path); - std::wstring widePath(ret->path, ret->path + strlen(ret->path)); - ret->findHandle = FindFirstFileExW(widePath.c_str(), FindExInfoStandard, &ret->data, FindExSearchNameMatch, NULL, 0); + ret->path = path + std::wstring(L"\\*"); + ret->startPath = path; + + ret->findHandle = FindFirstFileExW(ret->path.c_str(), FindExInfoStandard, &ret->data, FindExSearchNameMatch, NULL, 0); if (!ret->findHandle || ret->findHandle == INVALID_HANDLE_VALUE) { delete ret; return NULL; @@ -259,29 +70,20 @@ bool EbrFSDirReader::readNext(EbrDir* curDir, EbrDirEnt* ent) { return false; } - // Note that we're doing wcslen here, which is number of characters. If we've got continues - // in the stream then we may truncate the buffer. - std::string conv(data.cFileName, data.cFileName + wcslen(data.cFileName)); - strcpy_s(ent->fileName, conv.c_str()); - char tmpPath[4096]; - strcpy_s(tmpPath, startPath); - strcat_s(tmpPath, "//"); - strcat_s(tmpPath, conv.c_str()); + const std::string filename = Strings::WideToNarrow(std::wstring(data.cFileName)); + strcpy_s(ent->fileName, filename.c_str()); ent->isDir = (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY; return true; } EbrDir* EbrOpenDir(const char* path) { CPathMapper map(path); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return NULL; - } - if (!map.MappedPath()) + if (!map) { return NULL; + } - EbrDirReader* fsReader = EbrFSDirReader::open(fixedName); + EbrDirReader* fsReader = EbrFSDirReader::open(map.MappedPath()); if (!fsReader) { return NULL; } @@ -302,75 +104,71 @@ void EbrCloseDir(EbrDir* pDir) { delete pDir; } -bool EbrIsDir(const char* path) { - CPathMapper map(path); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return NULL; - } - - std::wstring unicodePath(map.MappedPath(), map.MappedPath() + strlen(map.MappedPath())); +static bool _EbrIsDir(const wchar_t* path) { WIN32_FILE_ATTRIBUTE_DATA fileAttribData; - if (GetFileAttributesExW(unicodePath.c_str(), GetFileExInfoStandard, &fileAttribData)) { + if (GetFileAttributesExW(path, GetFileExInfoStandard, &fileAttribData)) { if ((fileAttribData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY) return true; } return false; } +bool EbrIsDir(const char* path) { + CPathMapper map(path); + + if (!map) { + return NULL; + } + + return _EbrIsDir(map); +} + int EbrStat(const char* filename, struct stat* ret) { +#ifdef _USE_32BIT_TIME_T CPathMapper map(filename); - char* fixedName = map.FixedPath(); - if (!fixedName) { + if (!map) { + TraceError(TAG, L"EbrStat failure!"); return -1; } - if (EbrIsDir(filename)) { + if (_EbrIsDir(map)) { memset(ret, 0, sizeof(struct stat)); ret->st_size = 0; ret->st_mode = 0x1B6 | 0040000; return 0; } - if (!map.MappedPath()) { - TraceError(TAG, L"EbrStat failure!"); - return -1; - } - - return stat(map.MappedPath(), ret); + return _wstat32(map, reinterpret_cast(ret)); +#else + // This is from stat.h, unfortunately, it doesn't define the wstat apis for non 32-bit time_t + // essentially on our platform, EbrStat is same as EbrStat64i32. + // keeping this #ifdef just in case. + return EbrStat64i32(filename, reinterpret_cast(ret)); +#endif } int EbrStat64i32(const char* filename, struct _stat64i32* ret) { CPathMapper map(filename); - char* fixedName = map.FixedPath(); - if (!fixedName) { + if (!map) { + TraceError(TAG, L"EbrStat failure!"); return -1; } - if (EbrIsDir(filename)) { + if (_EbrIsDir(map)) { memset(ret, 0, sizeof(struct _stat64i32)); ret->st_size = 0; ret->st_mode = 0x1B6 | 0040000; return 0; } - if (!map.MappedPath()) { - TraceError(TAG, L"EbrStat failure!"); - return -1; - } - - return _stat64i32(map.MappedPath(), ret); + return _wstat64i32(map, ret); } int EbrAccess(const char* file, int mode) { CPathMapper map(file); - char* fixedName = map.FixedPath(); - if (!fixedName) { - return -1; - } - if (!map.MappedPath()) + if (!map) return -1; - return _access(map.MappedPath(), mode); + return _waccess(map, mode); } diff --git a/Frameworks/Starboard/EbrFile.cpp b/Frameworks/Starboard/EbrFile.cpp index 93a483c532..7cf78dbc7b 100644 --- a/Frameworks/Starboard/EbrFile.cpp +++ b/Frameworks/Starboard/EbrFile.cpp @@ -37,7 +37,7 @@ static const wchar_t* TAG = L"EbrFile"; std::mutex EbrFile::s_fileMapLock{}; std::map> EbrFile::s_fileMap{}; -int EbrFile::s_maxFileId{0}; +int EbrFile::s_maxFileId{ 0 }; std::shared_ptr EbrFile::GetFile(int fid) { std::lock_guard guard(s_fileMapLock); @@ -80,17 +80,16 @@ int EbrOpen(const char* file, int mode, int share) { } int EbrOpenWithPermission(const char* file, int mode, int share, int pmode) { - std::shared_ptr fileToAdd; // Special random number device. Just a stub. fileToAdd = EbrDevRandomFile::CreateInstance(file, mode, share, pmode); - + if (!fileToAdd) { // Special file type for cached storage files. fileToAdd = EbrStorageFile::CreateInstance(file, mode, share, pmode); - } - + } + if (!fileToAdd) { // No more special types. Assume its a real file. fileToAdd = EbrIOFile::CreateInstance(file, mode, share, pmode); @@ -150,95 +149,40 @@ int EbrFflush(int fd) { } bool EbrRemoveEmptyDir(const char* path) { - return RemoveDirectoryA(CPathMapper(path)); + return RemoveDirectoryW(CPathMapper(path)); } bool EbrRename(const char* path1, const char* path2) { - return rename(CPathMapper(path1), CPathMapper(path2)) == 0; + return _wrename(CPathMapper(path1), CPathMapper(path2)) == 0; } bool EbrUnlink(const char* path) { - return _unlink(CPathMapper(path)) == 0; + return _wunlink(CPathMapper(path)) == 0; } -#define FSROOT "." #define mkdir _mkdir -char g_WritableFolder[2048] = "."; +std::wstring g_WritableFolder(L"."); -void EbrSetWritableFolder(const char* folder) { - strcpy_s(g_WritableFolder, folder); +void IwSetWritableFolder(const wchar_t* folder) { + g_WritableFolder = folder; + // recreate the default folders + CPathMapper::CreateDefaultPaths(); } -const char* EbrGetWritableFolder() { - return g_WritableFolder; +const wchar_t* IwGetWritableFolder() { + return g_WritableFolder.c_str(); } -bool EbrGetRootMapping(const char* dirName, char* dirOut, uint32_t maxLen) { - if (dirName == NULL) { - strcpy_s(dirOut, maxLen, FSROOT); - return true; - } - if (_stricmp(dirName, "Documents") == 0) { - sprintf_s(dirOut, maxLen, "%s\\Documents", g_WritableFolder); - mkdir(dirOut); - - char tmpDir[4096]; - strcpy_s(tmpDir, dirOut); - strcat_s(tmpDir, "\\Library"); - mkdir(tmpDir); - return true; - } - if (_stricmp(dirName, "Cache") == 0) { - sprintf_s(dirOut, maxLen, "%s\\cache", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "Library") == 0) { - sprintf_s(dirOut, maxLen, "%s\\Library", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "AppSupport") == 0) { - sprintf_s(dirOut, maxLen, "%s\\AppSupport", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "tmp") == 0) { - sprintf_s(dirOut, maxLen, "%s\\tmp", g_WritableFolder); - mkdir(dirOut); - return true; - } - if (_stricmp(dirName, "shared") == 0) { - sprintf_s(dirOut, maxLen, "%s\\shared", g_WritableFolder); - mkdir(dirOut); - return true; - } - static std::regex drive("[a-zA-Z]:"); - if (std::regex_match(dirName, drive)) { - sprintf_s(dirOut, maxLen, dirName); - return true; - } - sprintf_s(dirOut, maxLen, FSROOT "\\%s", dirName); - return true; +const std::wstring& _IwGetWritableFolder() { + return g_WritableFolder; } bool EbrMkdir(const char* path) { - return _mkdir(CPathMapper(path)) == 0; -} - -char* EbrGetcwd(char* buf, size_t len) { - strncpy_s(buf, len, CPathMapper::currentDir, len); - return buf; -} - -int EbrChdir(const char* path) { - CPathMapper::setCWD(path); - - return 0; + return _wmkdir(CPathMapper(path)) == 0; } int EbrChmod(const char* path, int mode) { - return _chmod(CPathMapper(path), mode); + return _wchmod(CPathMapper(path), mode); } #define PATH_SEPARATOR "/" @@ -279,4 +223,4 @@ bool EbrRemove(const char* path) { } } return false; -} \ No newline at end of file +} diff --git a/Frameworks/Starboard/EbrIOFile.cpp b/Frameworks/Starboard/EbrIOFile.cpp index 390eabb7b8..e8cef67004 100644 --- a/Frameworks/Starboard/EbrIOFile.cpp +++ b/Frameworks/Starboard/EbrIOFile.cpp @@ -19,10 +19,9 @@ #include - std::shared_ptr EbrIOFile::CreateInstance(const char* path, int mode, int share, int pmode) { int fid{}; - int result = _sopen_s(&fid, CPathMapper(path), mode, share, pmode); + int result = _wsopen_s(&fid, CPathMapper(path), mode, share, pmode); if (result != 0) { return nullptr; diff --git a/Frameworks/Starboard/PathMapper.cpp b/Frameworks/Starboard/PathMapper.cpp new file mode 100644 index 0000000000..2cc424bd3d --- /dev/null +++ b/Frameworks/Starboard/PathMapper.cpp @@ -0,0 +1,148 @@ +//****************************************************************************** +// +// Copyright (c) Microsoft. All rights reserved. +// +// This code is licensed under the MIT License (MIT). +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//****************************************************************************** + +#include "Starboard.h" +#include "ErrorHandling.h" +#include "StringHelpers.h" + +#include +#include + +#include "Platform/EbrPlatform.h" +#include "PathMapper.h" + +// utility function to tokenize string using delimiters +// d:\src/winobjc ==> d:, src, winobjc +// /src/winobjc ==> "", src, winobjc +// / ==> "" +// src ==> "src" + +static const std::wstring c_currentDir(L"."); +static const std::wstring c_UpDir(L".."); + +static std::list _TokenizeString(const std::wstring& str, const wchar_t* delims) { + std::list components; + + std::size_t start = 0; // start from 0 + std::size_t delimPos = str.find_first_of(delims); + + while (start != std::wstring::npos) { + components.emplace_back(str.substr(start, delimPos - start)); + start = str.find_first_not_of(delims, delimPos); + delimPos = str.find_first_of(delims, start); + } + return components; +} + +// converts any ? to +, we may add more later if needed for compat + +static void _EscapeIllegalPathCharacters(std::wstring& str) { + std::size_t pos = str.find_first_of(L'?'); + while (pos != std::wstring::npos) { + str[pos] = L'+'; + pos = str.find_first_of(L'?', pos + 1); + } +} + +// normalize relative path +// 1 remove any "." or "" components +// 2 remove any ".." and preceeding component +// 3 if components becomes empty, return "." + +static void _NormalizeRelativePathComponents(std::list& components) { + for (auto it = components.begin(); it != components.end();) { + if (*it == c_currentDir || it->empty()) { + it = components.erase(it); + continue; + } else if (*it == c_UpDir) { + // remove previous component if it exists + if (it == components.begin()) { + // error, no previous directory exists + components.clear(); + break; + } + // we have to do this in case the last component is .. + // erase the previous component first because erase returns the next item + // which would be "..". + it = components.erase(--it); + it = components.erase(it); + } else { + ++it; + } + } + + if (components.empty()) { + components.emplace_back(c_currentDir); + } +} + +// the first component could have special meaning, we map it here +static const std::wstring c_specialFolders[] = { L"Documents", L"cache", L"Library", L"AppSupport", L"tmp", L"shared" }; + +std::wstring _MapPathRoot(const std::wstring& root) { + if (root == c_currentDir) { + return root; + } else if (root == L"~") { + return std::wstring(L".\\home"); + } + + if (root.length() == 2 && iswalpha(root[0]) && root[1] == L':') { + return root; + } + + for (auto folder : c_specialFolders) { + if (_wcsicmp(root.c_str(), folder.c_str()) == 0) { + return IwGetWritableFolder() + std::wstring(L"\\") + folder; + } + } + + return std::wstring(L".\\") + root; +} + +std::wstring _PathFromComponents(const std::list& components) { + std::wstring path = components.front(); + + auto it = components.begin(); + + std::for_each(++it, components.end(), [&path](const std::wstring& comp) { + path += std::wstring(L"\\"); + path += comp; + }); + _EscapeIllegalPathCharacters(path); + return path; +} + +CPathMapper::CPathMapper(const char* path) { + // create default paths once + static bool createDefaultPaths = CreateDefaultPaths(); + + std::wstring wpath = Strings::NarrowToWide(path); + std::list components = _TokenizeString(wpath, L"/\\"); + + _NormalizeRelativePathComponents(components); + components.front() = _MapPathRoot(components.front()); + mappedPath = _PathFromComponents(components); +} + +bool CPathMapper::CreateDefaultPaths() { + for (auto folder : c_specialFolders) { + _wmkdir((IwGetWritableFolder() + std::wstring(L"\\") + folder).c_str()); + } + // also create "Documents\\Library" + _wmkdir((IwGetWritableFolder() + std::wstring(L"\\Documents\\Library")).c_str()); + + return true; +} diff --git a/Frameworks/Starboard/PathMapper.h b/Frameworks/Starboard/PathMapper.h index 96b3745298..c3311e7c7b 100644 --- a/Frameworks/Starboard/PathMapper.h +++ b/Frameworks/Starboard/PathMapper.h @@ -14,31 +14,45 @@ // //****************************************************************************** +// +// CPathMapper is a convenience class used to perform some basic path mapping. +// +// 1. construct with a path with either leading or backward slashes (converted internally to / by appendPath) +// appendPath also mysteriously converts ~ to home. This isn't quite helping anyone since there is no 'home' directory on windows. +// we should simply drop this construct. +// 2. fixes up any relative paths +// a) /src/../src ==> /src +// b) /./src/winobjc/../ ==> /src +// c) see fixPath tests in pathmappertests.mm for other supported scenarios (and bugs) +// 3. The fixed up relative path is passed to convert path +// a) -> calls EbrGetRootMapping to replace special directories (ex: Documents, etc). See pathmappertests.mm. +// b) replaces / with \ +// c) removes any leading / introduced by fixPath +// 4. fixedPath is almost never used as real data, except in EbrOpenDir(), where its usage seems to be a bug. +// +// 5. mappedPath is the only real thing needed, so that is all we need to expose. +// +// + class CPathMapper { public: - char mappedPath[4096]; - char fixedPath[4096]; - bool fixedValid, mappedValid; + const std::wstring& MappedPath() const { + return mappedPath; + } - static char currentDir[4096]; + // constructor intentionally takes in char*, assumes UTF8 encoding. + CPathMapper(const char* path); - static void setCWD(const char* directory) { - if (directory[0] != '/') { - strcat_s(currentDir, directory); - } else { - strcpy_s(currentDir, directory); - } + operator const wchar_t*() const { + return mappedPath.c_str(); } - static void getCWD(char* directory) { - strcpy_s(directory, 4095, currentDir); + operator bool() const { + return !mappedPath.empty(); } - char* FixedPath(); - char* MappedPath(); + static bool CreateDefaultPaths(); - CPathMapper(const char* path); - operator const char*() { - return mappedPath; - } +private: + std::wstring mappedPath; }; diff --git a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp index 0860d1cd58..94747380a3 100644 --- a/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp +++ b/Frameworks/UIKit/StarboardXaml/ApplicationCompositor.cpp @@ -39,12 +39,11 @@ void InitializeApp() { initialized = true; // Set our writable and temp folders - char writableFolder[2048]; - size_t outLen; auto pathData = Windows::Storage::ApplicationData::Current->LocalFolder->Path; - wcstombs_s(&outLen, writableFolder, pathData->Data(), sizeof(writableFolder) - 1); - EbrSetWritableFolder(writableFolder); + IwSetWritableFolder(pathData->Data()); + char writableFolder[2048]; + size_t outLen; auto tempPathData = Windows::Storage::ApplicationData::Current->TemporaryFolder->Path; wcstombs_s(&outLen, writableFolder, tempPathData->Data(), sizeof(writableFolder) - 1); SetTemporaryFolder(writableFolder); diff --git a/Frameworks/include/Platform/EbrPlatform.h b/Frameworks/include/Platform/EbrPlatform.h index b81ce4b44c..57d8d8b60c 100644 --- a/Frameworks/include/Platform/EbrPlatform.h +++ b/Frameworks/include/Platform/EbrPlatform.h @@ -50,8 +50,6 @@ SB_EXPORT int EbrTruncate64(int fd, __int64 size); SB_EXPORT bool EbrRename(const char* path1, const char* path2); SB_EXPORT bool EbrUnlink(const char* path); SB_EXPORT bool EbrMkdir(const char* path); -SB_EXPORT char* EbrGetcwd(char* buf, size_t len); -SB_EXPORT int EbrChdir(const char* path); SB_EXPORT int EbrChmod(const char* path, int mode); @@ -84,10 +82,8 @@ SB_EXPORT int EbrGetTimeOfDay(struct EbrTimeval* curtime); SB_EXPORT double EbrGetMediaTime(); SB_EXPORT int EbrGetWantedOrientation(); -// maxLen should be MAX_PATH. Sorry Jordan. -SB_EXPORT bool EbrGetRootMapping(const char* dirName, char* dirOut, uint32_t maxLen); -SB_EXPORT const char* EbrGetWritableFolder(); -SB_EXPORT void EbrSetWritableFolder(const char* folder); +SB_EXPORT const wchar_t* IwGetWritableFolder(); +SB_EXPORT void IwSetWritableFolder(const wchar_t* folder); SB_EXPORT void EbrBlockIfBackground(); @@ -107,4 +103,3 @@ typedef struct { SB_EXPORT int EbrEventTimedMultipleWait(EbrEvent* events, int numEvents, double timeout, SocketWait* sockets); SB_EXPORT void EbrEventDestroy(EbrEvent event); - diff --git a/build/Starboard/dll/Starboard.def b/build/Starboard/dll/Starboard.def index 53148056e7..3b8023c58d 100644 --- a/build/Starboard/dll/Starboard.def +++ b/build/Starboard/dll/Starboard.def @@ -154,8 +154,6 @@ LIBRARY Starboard EbrRename EbrUnlink EbrMkdir - EbrGetcwd - EbrChdir EbrChmod EbrRemove EbrRemoveEmptyDir @@ -166,9 +164,6 @@ LIBRARY Starboard EbrGetTimeOfDay EbrGetMediaTime EbrGetWantedOrientation - EbrGetRootMapping - EbrGetWritableFolder - EbrSetWritableFolder EbrBlockIfBackground EbrEventInit EbrEventSignal @@ -178,5 +173,8 @@ LIBRARY Starboard EbrEventTimedMultipleWait EbrEventDestroy + IwGetWritableFolder + IwSetWritableFolder + ; REMOVE / CLEANUP C++ EXPORTS ?format@string@woc@@YA?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@PBDZZ diff --git a/build/Starboard/lib/StarboardLib.vcxproj b/build/Starboard/lib/StarboardLib.vcxproj index a38a21480d..ed19177903 100644 --- a/build/Starboard/lib/StarboardLib.vcxproj +++ b/build/Starboard/lib/StarboardLib.vcxproj @@ -22,7 +22,6 @@ - @@ -52,6 +51,8 @@ + + {2A00FC26-2ECF-4DF7-8ECF-2D18C5AC61C9} diff --git a/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj b/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj index cab68b0da0..bc08440a9f 100644 --- a/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj +++ b/build/Tests/UnitTests/Starboard/Starboard.UnitTests.vcxproj @@ -140,7 +140,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp -Wdeprecated-declarations SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;DEBUG=1;%(PreprocessorDefinitions) @@ -162,7 +162,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp -Wdeprecated-declarations SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;DEBUG=1;%(PreprocessorDefinitions) @@ -188,7 +188,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;%(PreprocessorDefinitions) MultiThreadedDLL @@ -214,7 +214,7 @@ false - $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) + $(StarboardBasePath)\Frameworks\include;$(StarboardBasePath)\include\xplat;$(StarboardBasePath)\tests\frameworks\include;$(StarboardBasePath)\tests\frameworks\gtest;$(StarboardBasePath)\tests\frameworks\gtest\include;$(StarboardBasePath)\;$(StarboardBasePath)\Frameworks\Starboard;$(StarboardBasePath)\tests\unittests\Tests.Shared;%(AdditionalIncludeDirectories) CompileAsObjCpp SB_IMPEXP=;NO_STUBS;_CRT_SECURE_NO_WARNINGS;%(PreprocessorDefinitions) MultiThreadedDLL @@ -237,6 +237,7 @@ + CompileAsObjC diff --git a/tests/unittests/Foundation/NSFileManagerTests.mm b/tests/unittests/Foundation/NSFileManagerTests.mm index 4e161be673..cb66ec1344 100644 --- a/tests/unittests/Foundation/NSFileManagerTests.mm +++ b/tests/unittests/Foundation/NSFileManagerTests.mm @@ -1,4 +1,4 @@ -//****************************************************************************** +//****************************************************************************** // // Copyright (c) Microsoft. All rights reserved. // @@ -213,3 +213,14 @@ // Verify data. ASSERT_OBJCEQ([content dataUsingEncoding:NSUTF8StringEncoding], [NSData dataWithContentsOfURL:destURL]); } + +TEST(NSFileManager, DirectoryWithUTF16Chars) { + wchar_t* specialFolder = L"oÖo winobjc"; + NSString* directoryName = [NSString stringWithCharacters:(const unichar*)specialFolder length:wcslen(specialFolder) * sizeof(wchar_t)]; + NSString* testPath = getPathToFile(directoryName); + + EXPECT_TRUE([[NSFileManager defaultManager] createDirectoryAtPath:testPath attributes:nil]); + EXPECT_TRUE([[NSFileManager defaultManager] fileExistsAtPath:testPath]); + EXPECT_TRUE([[NSFileManager defaultManager] removeItemAtPath:testPath error:nil]); + EXPECT_FALSE([[NSFileManager defaultManager] fileExistsAtPath:testPath]); +} diff --git a/tests/unittests/Starboard/PathMapperTests.mm b/tests/unittests/Starboard/PathMapperTests.mm new file mode 100644 index 0000000000..8b89a78c92 --- /dev/null +++ b/tests/unittests/Starboard/PathMapperTests.mm @@ -0,0 +1,103 @@ +//****************************************************************************** +// +// Copyright (c) 2016 Microsoft Corporation. All rights reserved. +// +// This code is licensed under the MIT License (MIT). +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//****************************************************************************** + +#include +#include "pathmapper.h" + +extern "C" void IwSetWritableFolder(const wchar_t*); +extern "C" const wchar_t* IwGetWritableFolder(); + +TEST(PathMapper, pathMapper) { + std::wstring writableFolder = IwGetWritableFolder(); + IwSetWritableFolder(L"d:\\temp"); + + CPathMapper mapper1("c:/users/winobjc-bot"); + EXPECT_STREQ(mapper1, L"c:\\users\\winobjc-bot"); + + CPathMapper mapper2("~/winobjc-bot"); + EXPECT_STREQ(mapper2, L".\\home\\winobjc-bot"); + + CPathMapper mapper3("Documents\\mydocuments/subfolder/.././"); + EXPECT_STREQ(mapper3, L"d:\\temp\\Documents\\mydocuments"); + + CPathMapper mapper4(""); + EXPECT_STREQ(mapper4, L"."); + + CPathMapper mapper5("src"); + EXPECT_STREQ(mapper5, L".\\src"); + + CPathMapper mapper6("c:/src/"); + EXPECT_STREQ(mapper6, L"c:\\src"); + + CPathMapper mapper7("/c:/src/"); + EXPECT_STREQ(mapper7, L"c:\\src"); + + CPathMapper mapper8("X:/src/"); + EXPECT_STREQ(mapper8, L"X:\\src"); + + CPathMapper mapper9("/Documents/src/"); + EXPECT_STREQ(mapper9, L"d:\\temp\\Documents\\src"); + + CPathMapper mapper10("/Documents/./"); + EXPECT_STREQ(mapper10, L"d:\\temp\\Documents"); + + CPathMapper mapper11("/Cache/test/."); + EXPECT_STREQ(mapper11, L"d:\\temp\\cache\\test"); + + CPathMapper mapper12("/library"); + EXPECT_STREQ(mapper12, L"d:\\temp\\Library"); + + CPathMapper mapper13("/AppSupport/././"); + EXPECT_STREQ(mapper13, L"d:\\temp\\AppSupport"); + + CPathMapper mapper14("tmp"); + EXPECT_STREQ(mapper14, L"d:\\temp\\tmp"); + + CPathMapper mapper15("/shared"); + EXPECT_STREQ(mapper15, L"d:\\temp\\shared"); + + CPathMapper mapper16("/AppSupport/.\\?/"); + EXPECT_STREQ(mapper16, L"d:\\temp\\AppSupport\\+"); + + IwSetWritableFolder(writableFolder.c_str()); +} + +TEST(PathMapper, RelativePathTests) { + CPathMapper mapper1("/.."); + EXPECT_STREQ(mapper1, L"."); + + CPathMapper mapper2("/./.."); + EXPECT_STREQ(mapper2, L"."); + + CPathMapper mapper3("/./src/../../.."); + // this test was failing before, it is a bug + EXPECT_STREQ(mapper3, L"."); + + CPathMapper mapper4("/src/../.."); + // this test was failing before, it is a bug + EXPECT_STREQ(mapper4, L"."); + + CPathMapper mapper5("/./src/../src/"); + // fails, returns //src + EXPECT_STREQ(mapper5, L".\\src"); + + CPathMapper mapper6("/./src/winobjc/.."); + EXPECT_STREQ(mapper6, L".\\src"); + + CPathMapper mapper7("/./././src/winobjc/../winobjc/.././../src/winobjc/.."); + // fails, returns //src + EXPECT_STREQ(mapper7, L".\\src"); +}