1806775 - "Show in Folder" silentry fail if the file path exceeds 255 char length or deeply nested folder.

Status: NEW

(Core :: Widget: Win32, defect, P3)

Description

[Alice0775 White]

Steps reproduce:

1. Download something to a folder with a long name
   e.g.
   1-1 Open https://ftp.mozilla.org/pub/firefox/nightly/2022/12/2022-12-20-21-46-32-mozilla-central/
   1-2 Right click on firefox-110.0a1.en-US.win64.zip and choose "Save Link As"
   1-3 After file picker opened choose a folder with long name(such as "d:\aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
   1-4 "Save"
2. Open download panel and execute "Show in Folder" after successfully downloaded

Actual results:
Nothing happens

Expected results:
Explorer should open

Comment 1

[Marco Bonardo [:mak]]

https://learn.microsoft.com/en-US/windows/win32/fileio/naming-a-file?redirectedfrom=MSDN
https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry

In editions of Windows before Windows 10 version 1607, the maximum length for a path is MAX_PATH, which is defined as 260 characters. In later versions of Windows, changing a registry key or using the Group Policy tool is required to remove the limit. See Maximum Path Length Limitation for full details.
For example, the maximum path on drive D is "D:\some 256-character path string<NUL>"

The Windows API has many functions that also have Unicode versions to permit an extended-length path for a maximum total path length of 32,767 characters. This type of path is composed of components separated by backslashes, each up to the value returned in the lpMaximumComponentLength parameter of the GetVolumeInformation function (this value is commonly 255 characters). To specify an extended-length path, use the "\?" prefix. For example, "\?\D:\very long path".

Starting in Windows 10, version 1607, MAX_PATH limitations have been removed from common Win32 file and directory functions. However, you must opt-in to the new behavior.

These limits of course change depending on the platform and filesystem, thus it's not trivial to satisfy them to the last bit.

There's a few notes about windows limits already in code like
https://searchfox.org/mozilla-central/rev/e6b709df9b93858364f02ab89f40d78762693db8/xpcom/io/nsLocalFileWin.cpp#2331-2334
And it's likely the open containing folder API is also not considering these cases.

Comment 2

[Marco Bonardo [:mak]]

The path in comment 0 is shorter than 260 chars, is that what you actually used for reproducing the bug, or just an example? (though once the file name is added onto it it will go over the limit and likely that causes the problem)

Comment 3

[Alice0775 White]

(In reply to Marco Bonardo [:mak] from comment #2)

The path in comment 0 is shorter than 260 chars, is that what you actually used for reproducing the bug, or just an example? (though once the file name is added onto it it will go over the limit and likely that causes the problem)

The bug is reproduced with the str. The file saves fine. However, the file name is truncated to firefox-1.zip instead of firefox-110.0a1.en-US.win64.zip.
And "Show in Folder" fails silently.

Comment 4

[Stephen A Pohl [:spohl] (OOO until 1/2)]

(In reply to Alice0775 White from comment #3)

(In reply to Marco Bonardo [:mak] from comment #2)

The path in comment 0 is shorter than 260 chars, is that what you actually used for reproducing the bug, or just an example? (though once the file name is added onto it it will go over the limit and likely that causes the problem)

The bug is reproduced with the str. The file saves fine. However, the file name is truncated to firefox-1.zip instead of firefox-110.0a1.en-US.win64.zip.
And "Show in Folder" fails silently.

So, the file name changing appears to be an intentional change from bug 1746139. However, "Show in Folder" should still work. This does seem like a Downloads Panel bug, as it should simply open the containing folder, no?

Comment 5

[Marco Bonardo [:mak]]

This is not related to drag & drop, the file was downloaded.
Afaict we sanitize filenames before the download, and then we call showContainingDirectory that calls file.parent.launch()... so there's surely to check if the file we call launch to is right, but then if it's right it will be a launch() problem. Let me check the first part.

Comment 6

[Marco Bonardo [:mak]]

So the exact path we pick is downloadsCmd_show that calls into lazy.DownloadsCommon.showDownloadedFile(file); that calls into file.reveal() that seems to silently fail, since I don't see an exception.
the nsIFile path is a long path \\?\D:\aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\100000000.bin (that seems correct, the file is there)

Comment 7

[Stephen A Pohl [:spohl] (OOO until 1/2)]

(In reply to Marco Bonardo [:mak] from comment #5)

This is not related to drag & drop, the file was downloaded.

You're right, I was doing triage and got this mixed up with bug 1806730.

(In reply to Marco Bonardo [:mak] from comment #6)

So the exact path we pick is downloadsCmd_show that calls into lazy.DownloadsCommon.showDownloadedFile(file); that calls into file.reveal() that seems to silently fail, since I don't see an exception.
the nsIFile path is a long path \\?\D:\aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\100000000.bin (that seems correct, the file is there)

I'm assuming you're looking into why file.reveal() fails at this time, or..?

Comment 8

[Marco Bonardo [:mak]]

(In reply to Stephen A Pohl [:spohl] from comment #7)

I'm assuming you're looking into why file.reveal() fails at this time, or..?

Not at the moment, I may return to it later but likely not before January.

Comment 9

[Stephen A Pohl [:spohl] (OOO until 1/2)]

Understood. Do you believe Widget:Win32 is the right component based on your current understanding?

Comment 10

[Marco Bonardo [:mak]]

reveal() is implemented in nsLocalFileWin.cpp that points to Core::XPCOM, but it's win32 specific, so I think it should be ok.