744413 - Firefox can't open local file with path longer than 254 characters in windows

Status: RESOLVED WONTFIX

(Core :: Networking: File, defect)

Description

[Edoardo Tirtarahardja]

Attachment: longPathFile.zip

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:10.0.2) Gecko/20100101 Firefox/10.0.2
Build ID: 20120215223356

Steps to reproduce:

Let say we have the following html files:
- C:\longPath240Char\parent.htm
- C:\longPath240Char\child1.htm
- C:\longPath240Char\1234567890\child2.htm

The parent.htm contains 2 frames those source './child1.html' and './1234567890\child2.htm' respectively.

The following method fails:
1. Double click file 'C:\longPath240Char\subFolder\child2.htm' from explorer.
2. Open the file 'C:\longPath240Char\subFolder\child2.htm' via menu 'File/Open file...'.
3. Open the file 'C:\longPath240Char\parent.htm'.


Actual results:

The result of each method is as followed:
1. FireFox load the file using windows method on accessing long path ('\\?\' prefix) in the urlbar (file://///%3F/C:/longPath240Char/1234567890/child2.htm) but it does not render the page.
2. A pop up 'Open File' with error message:
child1.htm
The above file name is invalid.
3. The 1st frame render the content of child1.htm correctly and the 2nd frame gives error message:
Firefox can't find the file at /c:/longPath240Char/1234567890/child2.



Expected results:

As windows I/O APIs are able to handle file with path longer than 254 characters by simply prefix it with '\\?\', then Firefox should be able to open 'C:\longPath240Char\1234567890\child2.htm' using all those method.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

IIRC, nsIFile trusts the MAX_PATH constant. However, under Windows, the APIs do not respect that constant, so we can indeed end up with "approximately" (that's a quote from the documentation) 32768 chars-long path names.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Edoardo: why did you classify this as "major"?

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Note: OS.File will not have this problem.

Comment 4

[Edoardo Tirtarahardja]

(In reply to David Rajchenbach Teller [:Yoric] from comment #2)
> Edoardo: why did you classify this as "major"?

Hi David,

Because there is no way for Firefox to open / render the 'child2.htm' in the described structure.

I have a test result reports that are automatically generated by our test instrument. The results are grouped according to the nested test groups, so in some test cases with long name, the report are nested down in structure that is longer than MAX_PATH. These html files are linked with each other, make it difficult for me to just move them around individually.

http://msdn.microsoft.com/en-us/library/aa365247%28VS.85%29.aspx#maxpath

As per above documentation, we should be able to use the unicode version of Windows API and using '\\?\' prefix to circumvent the MAX_PATH limitation.

BR //Edo

Comment 5

[David Teller [:Yoric] - still alive but not very active]

I believe the problem is that the original devs trusted Microsoft's documentation, which used to swear that MAX_PATH was the right constant, and then changed policy. Rooting this out would require a rather simple rewriting of our Windows uses of MAX_PATH. There are many of them, but most of the rewriting should be relatively straightforward:

http://mxr.mozilla.org/mozilla-central/ident?i=MAX_PATH&filter=

I do not have the time to work on it, but I believe that it is within grasp of a well-motivated newbie, so I will mark this bug as mentored.

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Just to be clear: I believe that the issue is that we often allocate buffers with length MAX_PATH before concatenating and/or receiving file names from the Windows APIs.

So, our first step is to look at any concatenation in nsLocalFileWin that uses such preallocated buffers, and then replace these concatenations with manipulations using our string classes.

Comment 7

[Thomas Powell]

Please assign this to me.  I'll start digging into it.

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Thomas, if you have any question, feel free to ping me on IRC. Also, there is a channel #windev of people who know more than me about Windows APIs.

Comment 9

[Thomas Powell]

I noticed this after David pointed out that passing NULL to the _wgetdcwd function should allocate the necessary string size. This is already being done with the pcwd pointer...  It uses the fixed-length buffer cwd first, but then makes the call with the NULL if the _wgetdcwd(,,MAX_PATH) call fails.

        WCHAR cwd[MAX_PATH];
        WCHAR * pcwd = cwd;
.
.
        if (!_wgetdcwd(drive, pcwd, MAX_PATH))
            pcwd = _wgetdcwd(drive, 0, 0);
        if (!pcwd)
            return NS_ERROR_OUT_OF_MEMORY;
        nsAutoString currentDir(pcwd);
        if (pcwd != cwd)
            free(pcwd);

Interestingly enough, if I sub in a larger MAX_PATH value, I end up with a "File Not Found" error when I finally try to retrieve the file at the end of the long path.

Comment 10

[David Teller [:Yoric] - still alive but not very active]

How do you "retrieve the file"?

Comment 11

[Thomas Powell]

Via the address bar or by linking through the directory tree in the browser window. 

Sorry, I'm thinking out loud a bit. I still need to change the path string to include \\?\ characters.

Comment 12

[Thomas Powell]

This fix is still in a "hack" stage, but I have it working by inserting a "\\?\" into a temp nsString before the calls that sent ConvertWinError a FILE_NOT_FOUND:

+    // XXX twpowell candidate for insert
+    nsString tempPath;
+    tempPath.Assign(name);
+    tempPath.Insert(L"\\\\?\\", 0);
+    // XXX end twpowell

My address bar path is the following, and I can navigate from c:\ down through the directory tree:
file:///c:/abbabbabbababababababababababababababababababababababbababababababababababbaabababab/abbabbababbabababbabbabbabbabababbabbababababababbabbabababbaababababababbabababab/abbabbabbababababbabbababababababababababababababababababababababababababababababbababa/appleappleappleappleappleapple.html

In the meantime, I'm assuming we'd want a test case built in xpcom/test/windows for this?

Comment 13

[David Teller [:Yoric] - still alive but not very active]

(In reply to Thomas Powell from comment #12)
> This fix is still in a "hack" stage, but I have it working by inserting a
> "\\?\" into a temp nsString before the calls that sent ConvertWinError a
> FILE_NOT_FOUND:
> 
> +    // XXX twpowell candidate for insert
> +    nsString tempPath;
> +    tempPath.Assign(name);
> +    tempPath.Insert(L"\\\\?\\", 0);
> +    // XXX end twpowell
> 
> My address bar path is the following, and I can navigate from c:\ down
> through the directory tree:
> file:///c:/
> abbabbabbababababababababababababababababababababababbababababababababababbaa
> bababab/
> abbabbababbabababbabbabbabbabababbabbababababababbabbabababbaababababababbaba
> babab/
> abbabbabbababababbabbabababababababababababababababababababababababababababab
> ababbababa/appleappleappleappleappleapple.html
> 
> In the meantime, I'm assuming we'd want a test case built in
> xpcom/test/windows for this?

This looks like as good a place as any.

Comment 14

[Jason Orendorff [:jorendorff]]

Thomas, are you still interested in working on this bug?

Comment 15

[Thomas Powell]

It's probably best if someone else takes this bug. I just started a new job and don't know when I'll get back to this.

Comment 16

[kiranmathewkoshy]

Hi.

I'm extremely new to Firefox development, just compiled FF for the 2nd time today.
Can you guide me on fixing this bug ?

Comment 17

[David Teller [:Yoric] - still alive but not very active]

@kiranmathewkoshy You should come and chat with us on IRC http://chat.mibbit.com/?server=irc.mozilla.org&channel=%23introduction.

Comment 18

[Michael Bao]

Hey all, new to Firefox development so hopefully I'll be able to be of help. Just got my build working and what not and started to look into this bug. The following is more or less just a brain dump so I know where to start off the next time I look at this. 

So first I took the example in the longPathFile.zip and ran it and based off what Thomas said earlier, I looked into nsLocalFile::Normalize() to see if the usage of MAX_PATH for cwd was affecting anything in this example. However, further debugging shows that not only did that path not get executed when loading the parent.htm file but the mWorkingPath was already set to cut off after "child2" so I traced it up on the call stack to nsPrincipal::CheckMayLoad. The frame's URI is put into the codebaseFileURL and I determined that the GetFile method being called was nsStandardURL::GetFile and realized that nsStandardURL::mSpec (the URI of the error) was able being truncated to not go past "child2."

From here, I'm going to keep on tracing up the call stack until I find the moment at which the string gets truncated (probably as stated before due to some buffer being set to MAX_PATH and the file path being copied into that).

Comment 19

[Michael Bao]

Another brain dump: So at this point I know that I'm pretty much looking for where the relative path becomes the absolute path and inside that chain of functions, I want to find where the absolute path becomes truncated.

So I further traced this bug a bit (at the moment I'm just trying to determine the cause of this particular bug with frames). So eventually I managed to track down the nsDocument::InitializeFrameLoader which is where the frame gets added to the mInitializableFrameLoaders array to eventually be loaded/displayed (at this point the URI has already been set and cuts off at child2).

So I traced this back even further to find NS_NewURI which eventually calls nsStandardURL::Resolve. Inside, I further tracked the change from relative to absolute to the call to nsBaseURLParser::ParseURL. Inside this function the parameter "const char *spec" turns from being relative to absolute although I have no idea how :) /end brain dump

Comment 20

[Michael Bao]

Woops. ^ All that brain dump is probably invalid since I assumed that my src path had *.htm in it (which it didn't) so back to the drawing board in trying to find where the problem is.

Comment 21

[Michael Bao]

Hm alright, narrowed this down. The error is coming from nsLocalFile::HasFileAttribute with the call to GetFileAttributesW. Since the file name is longer than MAX_PATH, the unicode function requires the path to be appended with \\?\ otherwise it results in failure and thus causes Firefox to throw the error. 

The easiest way around this is to go into nsLocalFile::Resolve and just add this line "mResolvedPath.Insert(L"\\\\?\\", 0);" (as mentioned by Thomas earlier) before the function returns.

Comment 22

[David Teller [:Yoric] - still alive but not very active]

Prepending with \\?\ makes sense and should work whether this is UNC or not, provided the path is not "/"-separated. I am not certain what we should do if the path is "/"-separated. We could either give up on it or convert it to "\"-separated.

I am a bit surprised that HasFileAttribute is the only place with issues, though.

Let me suggest the following steps:
- add your fix;
- continue investigating whether the error appears anywhere else;
- consider a Windows-specific nsIFile test with a long "/"-separated path.

Comment 23

[Michael Bao]

Hey, sorry I haven't updated this in awhile -- been quite busy but finally have some time to work on this again! 

1) I looked into the "/" vs "\"-separated file paths and didn't find an issue. I'm assuming that the potential issue stems from the statement: "File I/O functions in the Windows API convert "/" to "\" as part of converting the name to an NT-style name, except when using the "\\?\" prefix as detailed in the following sections." (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx). 

I ran a simple test with two frames:
<frame src=".\123\1234567890\child2.htm" name="child2">
<frame src="./123/1234567890/child2.htm" name="child2">
and they loaded up fine. Firefox seems to read them both as being "\"-separated (once it gets to nsLocalFile::Resolve both of them are separated by \).

2) I investigated using shortcuts and symbolic links to see if the error would appear. 
Symbolic Links: The error appears but is fixed by the same nsLocalFile::Resolve fix.
Shortcuts: Will never(?) be an issue as Windows won't let you create a shortcut with a name too long. 

3) I'm looking into writing the nsIFile test and I'm assuming it makes sense to put it under xpcom\tests\windows? What exactly would I be testing?

Comment 24

[Michael Bao]

[woops hit "save Changes" a bit too early]

Continuing 3) Would I just test to see if GetFileAttributesW fails or not after I create a nsLocalFile with the super long path? Also, how do I go about writing the test? Do I just create a new *.cpp with its own main and just have a function that does the test and then have main return 0 (success) or 1 (fail)? Is there anything else I would have to do? 

Cheers!

Comment 25

[David Teller [:Yoric] - still alive but not very active]

(In reply to Michael Bao from comment #23)
> Hey, sorry I haven't updated this in awhile -- been quite busy but finally
> have some time to work on this again! 
> 
> 1) I looked into the "/" vs "\"-separated file paths and didn't find an
> issue. I'm assuming that the potential issue stems from the statement: "File
> I/O functions in the Windows API convert "/" to "\" as part of converting
> the name to an NT-style name, except when using the "\\?\" prefix as
> detailed in the following sections."
> (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.
> 85%29.aspx). 

Basically, yes.

> I ran a simple test with two frames:
> <frame src=".\123\1234567890\child2.htm" name="child2">
> <frame src="./123/1234567890/child2.htm" name="child2">
> and they loaded up fine. Firefox seems to read them both as being
> "\"-separated (once it gets to nsLocalFile::Resolve both of them are
> separated by \).

What about if you try to open a file using the File:Open dialog? I suspect that it's not going to be converted.

> 2) I investigated using shortcuts and symbolic links to see if the error
> would appear. 
> Symbolic Links: The error appears but is fixed by the same
> nsLocalFile::Resolve fix.
> Shortcuts: Will never(?) be an issue as Windows won't let you create a
> shortcut with a name too long. 

Again, what about the File:Open dialog?
What if the symbolic link uses a /-separated path?

> 3) I'm looking into writing the nsIFile test and I'm assuming it makes sense
> to put it under xpcom\tests\windows? What exactly would I be testing?

I would have said xpcom/tests/unit/test_localfile.js

> Would I just test to see if GetFileAttributesW fails or not after I create a nsLocalFile with the super long path? Also, how do I go about writing the test? Do I just create a new *.cpp with its own main and just have a function that does the test and then have main return 0 (success) or 1 (fail)? Is there anything else I would have to do?

I'd extend the above JavaScript test to, essentially, create a nsIFile with a super long path and check if, for instance, exists() works correctly.

Comment 26

[Michael Bao]

1) Using the File:Open dialog - Tried getting "/"-separated paths using the File:Open dialog but couldn't (Windows 8). Tried manually typing out the file path to be "C:/ff/parent.htm" but the dialog would not recognize that as being a legitimate file path. Using the Open File dialog to load up a file with an extremely long path will automatically make it use the short-hand version so that shouldn't be an issue.

2) Symbolic Link "/"-Separated path - I used the same frame test as before but changed the 123 to 1234 where 1234 is a symbolic link to 123.
<frame src=".\1234\1234567890\child2.htm" name="child2">
<frame src="./1234/1234567890/child2.htm" name="child2">
and both cases work fine.

3) Unit Test - so inside test_localfile.js I created a new function test_windows_long_filename() and called it inside run_test. My function looks like this:
function test_windows_long_filename() 
{
  var file = new LocalFile("C:\\12345678901234567890123456789012345678901234567890123 \
    456789012345678901234567890123456789012345678901234567890123456789012345678901 \
    234567890123456789012345678901234567890123456789012345678901234567890123456789 \
    0123456789012345678901234567890\\1234567890\\parent.htm");
  file.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0644);   
  do_check_true(file.exists());
  if (file.exists())
    file.remove(true);
}

Does that look about right? How would I be able to run the test to make sure it works?

Comment 27

[Michael Bao]

I don't think I'll have much time to work on this anymore, sorry!

Comment 28

[Josh Matthews [:jdm]]

Michael, sorry that nobody responded to your question about unit tests - you can run it with |./mach xpcshell-test xpcom/tests/unit/test_localfile.js|.

Comment 29

[David Teller [:Yoric] - still alive but not very active]

Sorry for the silence, Michael. Your messages were lost in a deluge of other bugmail.
If you ever need to get in touch with someone (at least me), the best way is to use the "Need more information from" field and/or to ping the person on irc.

Comment 30

[Anup Kumar [:reznord]]

Hi,

I am interested in working on this bug. So please can you assing this bug to me.

Thanks in advance,

Regards,
A.Anup

Comment 31

[David Teller [:Yoric] - still alive but not very active]

Have you been working on that bug, Anup?

Comment 32

[Anup Kumar [:reznord]]

I have tried in some ways but none worked for me. Got some problem with my system. Will contact if I have any problem after I fix my system

Sorry for the delay

Comment 33

[sam63]

You can use long path tool for such problems as well.

Comment 34

[Moritz Brunner [:mb]]

Hi,
can anyone assign me to this bug?
One question, only method 3 of comment 1 isn't already fixed, right?

Comment 35

[David Teller [:Yoric] - still alive but not very active]

Hi Moritz,

Thanks for offering to contribute. I'm completely out of touch with this (I offered to mentor this bug in 2013) so I'm probably not the best person for the task anymore. Maybe someone from the Necko team would be better.

Comment 36

[Moritz Brunner [:mb]]

ok, but can you please assign me?

Comment 37

[Moritz Brunner [:mb]]

I tried to reproduce the bug, but I can't.I think the bug is already fixed.
Can anyone confirm this?

Comment 38

[Valentin Gosu [:valentin] (he/him)]

Attachment: test.patch

I can still reproduce this issue on Windows 10 using the attached patch.

run with ./mach test xpcom/tests/unit/test_localfile.js

I haven't looked into it very much, but it could be due to us observing MAX_PATH in nsLocalFileWin.cpp, or by a filesytem limitation described at:
https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx

If it's the second one (which it probably is), this bug is out of our reach.

Comment 39

[Moritz Brunner [:mb]]

Thank you for answering.
Did you fixed the attached test?Because inside parent.htm someone forgot to add the ".htm" suffix to the filename of the file "child2.htm" and if you fix it you will see that there is no bug anymore.

Comment 40

[Moritz Brunner [:mb]]

Sorry forget comment 39, I misunderstood that you uploaded a patch file...

Comment 41

[Masatoshi Kimura [:emk]]

FYI, Windows 10 Anniversary Update is going to add a new Group Policy and a manifest entry to unlock the MAX_PATH limit.
https://stackoverflow.com/questions/27680647/does-max-path-issue-still-exists-in-windows-10

Comment 42

[Moritz Brunner [:mb]]

Attachment: Bug 744413 - Fixed MAX_PATH limit.

Review commit: https://reviewboard.mozilla.org/r/194156/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194156/

Comment 43

[:Paolo Amadini]

Comment on attachment 8922981 [details]
Bug 744413 - Fixed MAX_PATH limit.

https://reviewboard.mozilla.org/r/194156/#review199224

We can't just prepend "\\?\" to all paths because this bypasses a number of normalizations and may introduce defects and security issues.

The approach from comment 41 is more likely to give correct results, but files with long paths are still not easily openable by many other applications. It's still better to enforce the limit for files we create, for example when downloading.

The only practical solution at the moment could be to waive the limit when using the "file:" protocol, that is read-only, although this would still require a security review to make sure things like backreferences and special characters are handled correctly.

Comment 44

[:Paolo Amadini]

Moving the bug to the component "Networking: File" as I think the only practical solution is necessarily limited to the "file:" protocol. Doing this generically for all the product would be too much work and unnecessary risk for now. If doing "file:" only also turns out to be risky, feel free to mark as WONTFIX. If the scenario of Windows changes we can always re-evaluate.

Comment 45

[Moritz Brunner [:mb]]

Probably removing the restriction from the "file:" protocol wouldn't make much sense because even Windows' file explorer can't handle files with long paths without throwing errors. For example Windows 10's file explorer can't open a file with a long path by double-clicking on it - at least on my system -, for that you need to use the "Open" in the context menu. Furthermore long local paths are seldom a problem as Firefox supports Windows' short names, for example folder "0123456789" becomes "012345~1".

Comment 46

[Dragana Damjanovic [:dragana]]

Valentin, can you take a look. I think this should be WONTFIX, I just want you to confirm.

Comment 47

[Valentin Gosu [:valentin] (he/him)]

I agree. This is hardly an issue at the moment.

Comment 48

[Mr. Beedell, Roke Julian Lockhart (RJLB)]

Attachment: A Screencast Of 139.0.4 Failing To Access A Path > Windows' MAX_PATH

I think this should be reconsidered, since MAX_PATH is oft bypassed nowadays. One example is how this prevents me accessing the paths that WSL2 exposes via file://///wsl.localhost/ (which are solely constrained to 4096 characters by Linux's PATH_MAX, under normal operation).

Environment

file:///$Env:ProgramFiles/WindowsApps/Mozilla.Firefox_139.0.4.0_x64__n80bbvh6b1yt2, from 9NZVDKPMR9RD.