-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
file: make _wopen() and _open() able to handle long legacy DOS paths #13512
Conversation
469dd66
to
d2be1b2
Compare
prepending |
@jay that modified path is only passed into _open and _wopen. Are you seeing otherwise? As far as I can tell after it is used by those functions it is no longer used anywhere else. |
I agree but I'm saying the logic is wrong. You can't assume that if a user specifies a path with a drive letter and colon that it is not going to need some interpretation. c:\foo/bar/../baz is not |
Right, it disables the path parsing, but it does not matter, the limitation The simple code with the existing 259 char path: #include <fcntl.h>
#include <io.h>
#include <stdio.h>
#include <string.h>
void win_open(const char *path, const char *msg) {
int fh1 = _open( path, _O_RDONLY ); // C4996
if(fh1 == -1) {
printf("Open %s failed on file ", msg);
perror("");
} else {
printf("Open %s succeeded on input file\n", msg);
_close(fh1);
}
}
int main(void) {
const char *path259_l = "\\\\?\\C:\\Work\\solid\\third_party\\curl\\tests\\log\\verylooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongdir\\dir\\..\\verylooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongfile3206.txt";
const char *parsed259_l = "\\\\?\\C:\\Work\\solid\\third_party\\curl\\tests\\log\\verylooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongdir\\verylooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongfile3206.txt";
win_open(path259_l + 4, "259 legacy");
win_open(path259_l, "259 long");
win_open(parsed259_l + 4, "259 parsed legacy");
win_open(parsed259_l, "259 parsed long");
} The output:
If the unparsed path in the argument exceeds 259 chars, it does not matter, neither that path, nor \\?\... path can be opened. So, \\?\ does not make it worse. |
As for forward slashes, curl translates them to backward slashes before |
d2be1b2
to
6589436
Compare
It does matter. I think it's wrong to prepend |
Not necessarily true, right. I don't assume anything. I just made it better in some cases, and not worse in other cases. file://localhost/c:/very-long-path and file:///c:/very-long-path now work and this is good for curl. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test fails in several windows CI jobs.
Windows builds don't seem to translate / to \ file paths. Will loot for the best solution, maybe just will just replace / with \ in the changed function. Weird, it worked locally at me. |
- Add a helper function for the Windows file wrapper functions that will normalize a long path (or a filename in a long path) and add the prefix `\\?\` so that Windows will access the file. Prior to this change if a filename (when normalized internally by Windows to its full path) or a path was longer than MAX_PATH (260) then Windows would not open the path, unless it was already normalized by the user and had the `\\?\` prefix prepended. The `\\?\` prefix could not be passed to file:// so for example something like file://c:/foo/bar/filename255chars could not be opened prior to this change. There's some code in tool_doswin that will need to be modified as well to further remove MAX_PATH (aka PATH_MAX) limitation. Ref: curl#8361 Ref: curl#13512 Ref: https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats Ref: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation Closes #xxxx
I'm proposing #13522 to address this issue with a normalization check and cover all the file functions that we wrap. |
I will check #13522 in WinRT on Monday. Long paths are usual in WinRT. |
Closing in favor of the other PR. |
Issue: #8361
Perl in
C:\Program Files\Git\usr\bin
supports long paths w/o any manifest or registry changes. Why not let curl support long paths.BTW the solution offered by "5.13 long paths are not fully supported on Windows" in the known bugs does not work, because there is no way how to specify
//?/
infile://
. RFC 8089 excludes such paths\\?\
and\\.\
.file://localhost///?/C:/foo
orfile://localhost/\\?\C:\foo
are also invalid URLs.