Skip to content

Commit 6e621f4

Browse files
mgallienmgallien
authored
Merge pull request #9077 from nextcloud/bugfix/noid/max-path
fix(filesystem/win32): make use of long paths where possible
2 parents 9ecd746 + 7a1e8fa commit 6e621f4

File tree

3 files changed

+45
-27
lines changed

3 files changed

+45
-27
lines changed

‎src/common/filesystembase.cpp‎

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,12 @@ void FileSystem::setFileReadOnly(const QString &filename, bool readonly)
106106
return;
107107
}
108108

109-
const auto windowsFilename = QDir::toNativeSeparators(filename);
110-
const auto fileAttributes = GetFileAttributesW(windowsFilename.toStdWString().c_str());
109+
const auto windowsFilename = longWinPath(filename);
110+
const auto rawWindowsFilename = reinterpret_cast<const wchar_t *>(windowsFilename.utf16());
111+
const auto fileAttributes = GetFileAttributesW(rawWindowsFilename);
111112
if (fileAttributes == INVALID_FILE_ATTRIBUTES) {
112113
const auto lastError = GetLastError();
113-
auto errorMessage = static_cast<char*>(nullptr);
114-
if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
115-
nullptr, lastError, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errorMessage, 0, nullptr) == 0) {
116-
qCWarning(lcFileSystem()) << "GetFileAttributesW" << windowsFilename << (readonly ? "readonly" : "read write") << errorMessage;
117-
} else {
118-
qCWarning(lcFileSystem()) << "GetFileAttributesW" << windowsFilename << (readonly ? "readonly" : "read write") << "unknown error" << lastError;
119-
}
114+
qCWarning(lcFileSystem()).nospace() << "GetFileAttributesW failed, action=" << (readonly ? "readonly" : "read write") << " filename=" << windowsFilename << " lastError=" << lastError << " errorMessage=" << Utility::formatWinError(lastError);
120115
return;
121116
}
122117

@@ -127,15 +122,9 @@ void FileSystem::setFileReadOnly(const QString &filename, bool readonly)
127122
newFileAttributes = newFileAttributes & (~FILE_ATTRIBUTE_READONLY);
128123
}
129124

130-
if (SetFileAttributesW(windowsFilename.toStdWString().c_str(), newFileAttributes) == 0) {
125+
if (SetFileAttributesW(rawWindowsFilename, newFileAttributes) == 0) {
131126
const auto lastError = GetLastError();
132-
auto errorMessage = static_cast<char*>(nullptr);
133-
if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS,
134-
nullptr, lastError, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), errorMessage, 0, nullptr) == 0) {
135-
qCWarning(lcFileSystem()) << "SetFileAttributesW" << windowsFilename << (readonly ? "readonly" : "read write") << errorMessage;
136-
} else {
137-
qCWarning(lcFileSystem()) << "SetFileAttributesW" << windowsFilename << (readonly ? "readonly" : "read write") << "unknown error" << lastError;
138-
}
127+
qCWarning(lcFileSystem()).nospace() << "SetFileAttributesW failed, action=" << (readonly ? "readonly" : "read write") << " filename=" << windowsFilename << " lastError=" << lastError << " errorMessage=" << Utility::formatWinError(lastError);
139128
}
140129

141130
if (!readonly) {
@@ -773,10 +762,11 @@ bool FileSystem::setAclPermission(const QString &unsafePath, FolderPermissions p
773762
auto neededLength = 0ul;
774763

775764
const auto path = longWinPath(unsafePath);
765+
const auto rawPath = reinterpret_cast<const wchar_t *>(path.utf16());
776766

777767
const auto safePathFileInfo = QFileInfo{path};
778768

779-
if (!GetFileSecurityW(path.toStdWString().c_str(), info, nullptr, 0, &neededLength)) {
769+
if (!GetFileSecurityW(rawPath, info, nullptr, 0, &neededLength)) {
780770
const auto lastError = GetLastError();
781771
if (lastError != ERROR_INSUFFICIENT_BUFFER) {
782772
qCWarning(lcFileSystem) << "error when calling GetFileSecurityW" << path << Utility::formatWinError(lastError);
@@ -785,7 +775,7 @@ bool FileSystem::setAclPermission(const QString &unsafePath, FolderPermissions p
785775

786776
securityDescriptor.reset(new char[neededLength]);
787777

788-
if (!GetFileSecurityW(path.toStdWString().c_str(), info, securityDescriptor.get(), neededLength, &neededLength)) {
778+
if (!GetFileSecurityW(rawPath, info, securityDescriptor.get(), neededLength, &neededLength)) {
789779
qCWarning(lcFileSystem) << "error when calling GetFileSecurityW" << path << Utility::formatWinError(GetLastError());
790780
return false;
791781
}
@@ -805,7 +795,7 @@ bool FileSystem::setAclPermission(const QString &unsafePath, FolderPermissions p
805795
PSID sid = nullptr;
806796
if (!ConvertStringSidToSidW(L"S-1-5-32-545", &sid))
807797
{
808-
qCWarning(lcFileSystem) << "error when calling ConvertStringSidToSidA" << path << Utility::formatWinError(GetLastError());
798+
qCWarning(lcFileSystem) << "error when calling ConvertStringSidToSidW" << path << Utility::formatWinError(GetLastError());
809799
return false;
810800
}
811801

@@ -886,24 +876,24 @@ bool FileSystem::setAclPermission(const QString &unsafePath, FolderPermissions p
886876
for (const auto &oneEntry : childFiles) {
887877
const auto childFile = joinPath(path, oneEntry);
888878

889-
const auto &childFileStdWString = childFile.toStdWString();
890-
const auto attributes = GetFileAttributes(childFileStdWString.c_str());
879+
const auto rawChildFile = reinterpret_cast<const wchar_t *>(childFile.utf16());;
880+
const auto attributes = GetFileAttributesW(rawChildFile);
891881

892882
// testing if that could be a pure virtual placeholder file (i.e. CfApi file without data)
893883
// we do not want to trigger implicit hydration ourself
894884
if ((attributes & FILE_ATTRIBUTE_SPARSE_FILE) != 0) {
895885
continue;
896886
}
897887

898-
if (!SetFileSecurityW(childFileStdWString.c_str(), info, &newSecurityDescriptor)) {
888+
if (!SetFileSecurityW(rawChildFile, info, &newSecurityDescriptor)) {
899889
qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << childFile << Utility::formatWinError(GetLastError());
900890
return false;
901891
}
902892
}
903893
}
904894

905-
if (!SetFileSecurityW(QDir::toNativeSeparators(path).toStdWString().c_str(), info, &newSecurityDescriptor)) {
906-
qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << QDir::toNativeSeparators(path) << Utility::formatWinError(GetLastError());
895+
if (!SetFileSecurityW(rawPath, info, &newSecurityDescriptor)) {
896+
qCWarning(lcFileSystem) << "error when calling SetFileSecurityW" << path << Utility::formatWinError(GetLastError());
907897
return false;
908898
}
909899

‎src/csync/std/c_time.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ int c_utimes(const QString &uri, const time_t time)
3030
#include <errno.h>
3131
#include <wtypes.h>
3232

33+
#include "common/filesystembase.h"
34+
3335
constexpr long long CSYNC_SECONDS_SINCE_1601 = 11644473600LL;
3436
constexpr long long CSYNC_USEC_IN_SEC = 1000000LL;
3537

@@ -50,11 +52,11 @@ int c_utimes(const QString &uri, const time_t time)
5052
FILETIME filetime;
5153
HANDLE hFile = nullptr;
5254

53-
auto wuri = uri.toStdWString();
55+
const auto wuri = reinterpret_cast<const wchar_t *>(OCC::FileSystem::longWinPath(uri).utf16());
5456

5557
UnixTimeToFiletime(time, &filetime);
5658

57-
hFile=CreateFileW(wuri.data(), FILE_WRITE_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
59+
hFile=CreateFileW(wuri, FILE_WRITE_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
5860
nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL+FILE_FLAG_BACKUP_SEMANTICS, nullptr);
5961
if(hFile==INVALID_HANDLE_VALUE) {
6062
switch(GetLastError()) {

‎test/testfilesystem.cpp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@
99

1010
#include <QtTest>
1111
#include <QTemporaryDir>
12+
#include <QTemporaryFile>
1213

1314
#include "common/filesystembase.h"
1415
#include "logger.h"
1516

1617
#include "libsync/filesystem.h"
1718

1819
using namespace OCC;
20+
using namespace Qt::StringLiterals;
1921
namespace std_fs = std::filesystem;
2022

2123
class TestFileSystem : public QObject
@@ -127,6 +129,30 @@ private Q_SLOTS:
127129
QCOMPARE(permissionsDidChange, false);
128130
}
129131
#endif
132+
133+
void testSetFileReadOnlyLongPath()
134+
{
135+
// this should be enough to hit Windows' default MAX_PATH limitation (260 chars)
136+
QTemporaryFile longFile(u"test"_s.repeated(60));
137+
138+
// QTemporaryFile::open actually creates the file
139+
QVERIFY(longFile.open());
140+
QVERIFY(!longFile.fileName().isEmpty());
141+
142+
QFileInfo fileInfo(longFile);
143+
const auto path = fileInfo.absoluteFilePath();
144+
145+
// ensure we start with a read/writeable directory
146+
QVERIFY(FileSystem::isWritable(path));
147+
148+
FileSystem::setFileReadOnly(path, true);
149+
// path should now be readonly
150+
QVERIFY(!FileSystem::isWritable(path));
151+
152+
FileSystem::setFileReadOnly(path, false);
153+
// path should now be writable aagain
154+
QVERIFY(FileSystem::isWritable(path));
155+
}
130156
};
131157

132158
QTEST_GUILESS_MAIN(TestFileSystem)

0 commit comments

Comments
 (0)