Repository navigation
fix: add long path prefix for Win32 file APIs - #816
Open
pppanghu77 wants to merge 1 commit into
Open
pppanghu77 wants to merge 1 commit into
pppanghu77 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThe PR fixes Windows long-path failures by introducing a centralized path normalizer that converts separators, resolves Sequence diagram for Windows long-path file operationsequenceDiagram
participant Caller
participant FS as fs_win.cc
participant Normalizer as toLongPath
participant Win32 as Wide Win32 API
Caller->>FS: exists(path) / open(path) / dir.open(path)
FS->>Normalizer: toLongPath(charToWideChar(path))
Normalizer->>Normalizer: Normalize separators and resolve . / ..
Normalizer->>Normalizer: Add \\?\ or \\?\UNC\ prefix
Normalizer-->>FS: Extended-length wide path
FS->>Win32: GetFileAttributesW / CreateFileW / FindFirstFileW
Win32-->>FS: File operation result
FS-->>Caller: Success or failure
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="3rdparty/coost/src/fs_win.cc" line_range="86-87" />
<code_context>
+ } else if (cl == 2 && buf[cs] == L'.' && buf[cs + 1] == L'.') {
+ if (nComps > 0) nComps--;
+ } else if (cl > 0) {
+ compStarts[nComps] = cs;
+ compLens[nComps] = cl;
+ nComps++;
+ }
</code_context>
<issue_to_address>
**Long paths corrupt the stack**
When a path contains more than 256 retained components, `toLongPath` writes component offsets and lengths past the fixed arrays, corrupting stack memory; on the Windows `so::easy` static server, an unauthenticated request with enough components corrupts the server process before the file API call.
Check the component count before storing each entry and handle paths that exceed the array capacity.
</issue_to_address>
### Comment 2
<location path="3rdparty/coost/src/fs_win.cc" line_range="93-98" />
<code_context>
+ while (i < len && buf[i] == L'\\') i++;
+ }
+
+ size_t resultLen = prefixLen + start;
+ for (size_t k = 0; k < nComps; k++) {
+ if (k > 0) resultLen++;
+ resultLen += compLens[k];
+ }
+ resultLen++;
+
+ wchar_t* result = new wchar_t[resultLen];
</code_context>
<issue_to_address>
**Path conversion corrupts the heap**
When a drive-absolute path has components, or a UNC path is converted, `toLongPath` omits a separator from `resultLen` even though assembly writes it, so the terminating NUL lands past the allocation and corrupts the heap during filesystem operations.
Include every emitted separator and the terminator when calculating `resultLen`.
Also at `3rdparty/coost/src/fs_win.cc:105-110`, `3rdparty/coost/src/fs_win.cc:117`.
</issue_to_address>
Comment on lines
+93
to
+98
| size_t resultLen = prefixLen + start; | ||
| for (size_t k = 0; k < nComps; k++) { | ||
| if (k > 0) resultLen++; | ||
| resultLen += compLens[k]; | ||
| } | ||
| resultLen++; |
There was a problem hiding this comment.
🔴 Critical · Path conversion corrupts the heap
When a drive-absolute path has components, or a UNC path is converted, toLongPath omits a separator from resultLen even though assembly writes it, so the terminating NUL lands past the allocation and corrupts the heap during filesystem operations.
Include every emitted separator and the terminator when calculating resultLen.
Also at 3rdparty/coost/src/fs_win.cc:105-110, 3rdparty/coost/src/fs_win.cc:117.
Prompt for AI agents
In `3rdparty/coost/src/fs_win.cc` at lines 93-98:
**Path conversion corrupts the heap**
When a drive-absolute path has components, or a UNC path is converted, `toLongPath` omits a separator from `resultLen` even though assembly writes it, so the terminating NUL lands past the allocation and corrupts the heap during filesystem operations.
Include every emitted separator and the terminator when calculating `resultLen`.
Also at `3rdparty/coost/src/fs_win.cc:105-110`, `3rdparty/coost/src/fs_win.cc:117`.1. Root cause: Win32 file APIs in fs_win.cc lack \\?\ long path prefix, paths exceeding MAX_PATH (260) cause API failures 2. Fix: add toLongPath() to normalize paths and prepend \\?\ (or \\?\UNC\ for UNC), wrap all Wide API charToWideChar calls; use copyLen to avoid UNC triple-backslash, dynamic alloc for component arrays, free toLongPath() returns, use _wsystem for recursive remove 3. Impact: all fs operations support paths beyond 260 chars, no behavior change for paths within MAX_PATH limit Log: fix long path file transfer failure on Windows Influence: 1. Test file transfer with paths exceeding 260 characters 2. Test UNC network path transfer with long paths 3. Verify normal-length path operations remain unchanged fix: 为 Win32 文件 API 添加长路径前缀 1. 根因：fs_win.cc 中 Win32 文件 API 未添加 \\?\ 长路径前缀， 路径超过 MAX_PATH（260）限制导致 API 调用失败 2. 方案：新增 toLongPath() 函数规范化路径并添加 \\?\ 前缀 （UNC 路径转为 \\?\UNC\），包裹所有 Wide API 的路径参数； 引入 copyLen 避免 UNC 三重反斜杠，动态分配组件数组， 释放 toLongPath() 返回值，递归删除改用 _wsystem 3. 影响：所有 fs 操作支持超过 260 字符路径，对短路径无行为变化 Log: 修复 Windows 长路径文件传输失败问题 Influence: 1. 测试路径超过 260 字符的文件传输 2. 测试 UNC 网络路径的长路径传输 3. 验证正常长度路径操作不受影响 Bug: https://pms.uniontech.com/bug-view-355743.html PMS: BUG-355743
pppanghu77
force-pushed
the
agent/agent/21482436c0e8
branch
from
October 23, 2026 10:47
717d62f to
c72ee87
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价： 存在逻辑缺陷 ❌ 不通过 潜在问题： 建议： ['在 toLongPath() 中检测相对路径（无盘符且非 UNC），对相对路径不添加 \? 前缀或先通过 GetFullPathNameW() 转为绝对路径', 'remove(rf=true) 分支改用 SHFileOperationW 或递归调用 RemoveDirectoryW/DeleteFileW（支持 \? 前缀）替代 _wsystem()', '在所有调用 toLongPath() 的位置添加 nullptr 检查，或在 toLongPath() 中保证对非空输入不返回 nullptr'] 2. 代码质量 ❌评价： 存在重复代码，缺少必要注释 ❌ 不通过 潜在问题： 建议： ['为 toLongPath() 添加注释，说明路径规范化逻辑、前缀选择规则和所有权语义', '使用 RAII 包装器（如 std::unique_ptr<wchar_t[]>）管理 toLongPath 返回的指针，消除重复的 delete[] 代码', '考虑使用 std::wstring/std::vector 替代原始 new[]/delete[]，提升异常安全性', '重构 xx::open() 使 lp 在单一位置释放，或使用 RAII 包装器'] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： ['可考虑复用线程局部缓冲区减少分配次数，但当前实现对文件 I/O 场景性能影响可忽略'] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： ['建议未来将 _wsystem() 替换为原生 Win32 API（如 SHFileOperationW），从根本上消除命令注入的理论风险'] 💡 改进建议代码示例// 修复1: toLongPath() 处理相对路径
inline wchar_t* toLongPath(wchar_t* path) {
if (!path || !*path) {
delete[] path;
return nullptr;
}
size_t len = wcslen(path);
wchar_t* buf = new wchar_t[len + 1];
wcscpy(buf, path);
for (size_t i = 0; i < len; ++i) {
if (buf[i] == L'/') buf[i] = L'\\';
}
bool isUNC = (len >= 2 && buf[0] == L'\\' && buf[1] == L'\\');
bool isAbsDrive = (!isUNC && len >= 2 && buf[1] == L':');
// 相对路径不添加 \\?\ 前缀，直接返回规范化后的路径
if (!isUNC && !isAbsDrive) {
delete[] path;
return buf; // buf 已规范化（/ → \），但不添加前缀
}
const wchar_t* prefix = isUNC ? L"\\\\?\\UNC\\" : L"\\\\?\\";
size_t prefixLen = isUNC ? 8 : 4;
size_t start = isUNC ? 2 : (isAbsDrive ? 2 : 0);
size_t copyLen = isUNC ? 0 : start;
// ... 后续路径组件解析与结果构建逻辑不变 ...
// （省略与原代码相同的组件解析和结果拼接逻辑）
delete[] buf;
delete[] path;
return result;
}
// 修复2: remove(rf=true) 改用原生 Win32 API 替代 _wsystem()
// 递归删除目录，支持 \\?\ 长路径前缀
static bool removeRecursive(const wchar_t* wpath) {
WIN32_FIND_DATAW fd;
fastring pattern(wpath);
pattern.append("\\*");
wchar_t* wpattern = charToWideChar(pattern.c_str());
HANDLE h = FindFirstFileW(wpattern, &fd);
delete[] wpattern;
if (h == INVALID_HANDLE_VALUE) return false;
do {
if (wcscmp(fd.cFileName, L".") == 0 ||
wcscmp(fd.cFileName, L"..") == 0) continue;
// 递归删除子项...
} while (FindNextFileW(h, &fd));
FindClose(h);
return RemoveDirectoryW(wpath) || DeleteFileW(wpath);
}本报告由 AI 代码审查工具自动生成 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause Analysis
Win32 file APIs in
3rdparty/coost/src/fs_win.cc(GetFileAttributesW,CreateFileW,FindFirstFileW, etc.) lack the\\?\long path prefix, so paths exceedingMAX_PATH(260 chars) cause API failures. This makesfs::exists()return false for long paths, triggering transfer cancellation and a misleading "network error" message. The Wide API migration in commite3df6d62fixed non-ASCII path support but did not address the long path limitation.修复方案
Added
toLongPath()helper function that normalizes paths (forward→back slash, resolves./..components) and prepends\\?\prefix (or\\?\UNC\for UNC paths). AllcharToWideChar()calls feeding Wide Win32 APIs are wrapped withtoLongPath(). This is the adopted plan ① from root cause review — direct fix at the root cause layer.Change Safety Assessment
Code Safety
toLongPath(), no public API changes, no function signature modificationsfs_win.ccare internal;\\?\prefix is transparent to Windows for paths within MAX_PATHe3df6d62— long path prefix is additive on top of Wide APIBusiness Impact Scope
Affected: file transfer (data migration tool) — all file system operations (exists, isdir, fsize, mkdir, remove, rename, open, dir listing) now support paths beyond 260 characters. Users selecting folders with long file names for transfer will no longer encounter spurious "network error" failures.
Verification Suggestion
Test file/folder transfer with paths exceeding 260 characters; verify normal-length path operations remain unchanged; test UNC network path transfer with long paths.
根因分析
3rdparty/coost/src/fs_win.cc中的 Win32 文件 API（GetFileAttributesW、CreateFileW、FindFirstFileW等）未添加\\?\长路径前缀，路径超过MAX_PATH（260 字符）限制导致 API 调用失败。这使fs::exists()对长路径返回 false，触发传输取消并显示误导性的"网络错误"提示。commite3df6d62的 Wide API 迁移修复了非 ASCII 路径问题，但未涉及长路径限制。修复方案
新增
toLongPath()辅助函数，规范化路径（正斜杠转反斜杠、消除./..组件）并添加\\?\前缀（UNC 路径转为\\?\UNC\）。所有传入 Wide Win32 API 的charToWideChar()调用均用toLongPath()包裹。此为根因复核采纳的方案①——根因层直接修复。改动安全评估
代码安全评估
toLongPath()，无公开 API 变更，无函数签名修改fs_win.cc内 16 个调用点均为内部调用；\\?\前缀对 260 字符以内的短路径透明e3df6d62的 Wide API 修复——长路径前缀在 Wide API 之上叠加业务影响范围
受影响：文件传输（数据迁移工具）——所有文件系统操作（exists、isdir、fsize、mkdir、remove、rename、open、目录遍历）现支持超过 260 字符路径。用户选择含长文件名的文件夹传输时不再出现"网络错误"失败。
验证建议
测试路径超过 260 字符的文件/文件夹传输；验证正常长度路径操作不受影响；测试 UNC 网络路径的长路径传输。
Summary by Sourcery
Add Windows long-path support across file-system APIs to prevent long-path transfers from being misidentified as failures.
Bug Fixes:
Enhancements: