Dragging a URL to windows file system can create files with too long filenames
Categories
(Firefox :: File Handling, defect, P3)
Tracking
()
People
(Reporter: tore, Unassigned, NeedInfo)
References
(Regression)
Details
(Keywords: parity-chrome, regression)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:127.0) Gecko/20100101 Firefox/127.0
Steps to reproduce:
When I visit https://github.com/paolosalvatori/servicebusexplorer
I want to save a copy of the URL into a local Windows 10 folder. I drag the URL and a shortcut is created. Great! That works!
Actual results:
But the file name is too long! Due to the length, If I try to double click on the URL to open the page in firefox, then Windows complains that can't/won't open it.
when trying to add the file in Git, also fails
Z:\GitHubTN\azure\Azure for .NET developers\Module 16 - Event and Message driven applications\30 - Azure Service Bus>git add .
error: open("Azure for .NET developers/Module 16 - Event and Message driven applications/30 - Azure Service Bus/paolosalvatori_ServiceBusExplorer_ The Service Bus Explorer allows users to connect to a Service Bus namespace and administer messaging entities in an easy manner. The tool provides advanced features like import_export functionality or the ability to.url"): Filename too long
error: unable to index file 'Azure for .NET developers/Module 16 - Event and Message driven applications/30 - Azure Service Bus/paolosalvatori_ServiceBusExplorer_ The Service Bus Explorer allows users to connect to a Service Bus namespace and administer messaging entities in an easy manner. The tool provides advanced features like import_export functionality or the ability to.url'
fatal: adding files failed
Expected results:
I would expect that FireFox would truncate the file name (based on the page title), so that the total length including the path is less than the max path/file length in Windows.
Google Chrome handles this much better , it limit the file name to
GitHub - paolosalvatori-ServiceBusExplorer- The Service Bus Explorer allows users to connect to a Service Bus namespace and a.url
While Firefox creates this URL:
paolosalvatori_ServiceBusExplorer_ The Service Bus Explorer allows users to connect to a Service Bus namespace and administer messaging entities in an easy manner. The tool provides advanced features like import_export functionality or the ability to.url
Comment 1•3 months ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::Widget: Win32' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
Comment 2•2 months ago
|
||
We actually do truncate the alleged filename to MAX_PATH
characters [0]... but that won't help when the file is in any directory, and especially not a directory whose path is itself 99 characters long. Chrome appears to truncate to approximately MAX_PATH/2
characters, which is probably a reasonable approach.
Note that, as a workaround, the file can be renamed from File Explorer by selecting it and pressing F2, or equivalently by right-clicking it and selecting "Rename".
Could it not adjust the filename based on the current path so that its total length and the path are within the limits?
When the file name gets too long, things start to break. You can't click on it, and you can't rename it in some cases, in some cases it can't be deleted.
so it's pretty hard to get rid of when the file name is too long.
All I want is to avoid any surprises, regardless of the current path depth :-)
Comment 4•2 months ago
|
||
Could it not adjust the filename based on the current path so that its total length and the path are within the limits?
Unfortunately, I don't believe it can. Windows doesn't tell us what the target path is; it just tells us that whatever we're dropping the object onto wants a file.
Comment 5•2 months ago
|
||
Ray, you moved this to file handling - where does the code that provides a filename to Windows for the drag/drop actually live? Although we could change the URL bar code to e.g. produce a shorter title in the x-moz-url
format, (a) that would not be great for other drags (that don't result in url files) and (b) that would not fix other drag sources.
I would have thought this stuff lives in widget's nsDragService
? I can totally believe it's somewhere else - but I'm not sure how to find out, and it sounds like you already dug into this a bit.
Comment 6•2 months ago
|
||
Actually, Marco found https://searchfox.org/mozilla-central/rev/91f6127b6f591da4037821791c345147d9a575da/widget/windows/nsDataObj.cpp#1203,1244 and that lives in widget. So I'm tentatively moving this back to widget but will keep the needinfo.
Comment 7•2 months ago
•
|
||
(In reply to :Gijs (he/him) from comment #5)
Ray, you moved this to file handling - where does the code that provides a filename to Windows for the drag/drop actually live?
The code you linked is the code that provides the filename to Windows, yes.
The code that provides the filename to that code, however, is highlighted in my previous comment...
... and exists in uriloader/exthandler/nsExternalHelperAppService.cpp
.
The functions Marco noted both call ValidateFilename(), which invokes nsIMIMEService::ValidateFileNameForSaving, which is implemented by nsExternalHelperAppService by invoking a different overload of ValidateFileNameForSaving, which invokes SanitizeFilename, which is the function I linked as [a], above.
I have no reason to believe that the calls in nsDataObj.cpp
are the only invocations of ValidateFileNameForSaving which could result in this issue.
Comment 8•2 months ago
|
||
Neil is probably best-placed to suggest what should happen here, then. validateFilenameForSaving
also does not take a target directory so it still cannot really reliably do better in terms of figuring out MAX_PATH
constraints. I would anticipate that if that helper ended up overwriting user-chosen filenames to make them fit in MAX_PATH / 2
, that would upset people.
What I suspect may need to happen is (a) allow consumers to pass an nsIFile
or similar for the containing path, if available (which I recognize it's not in this case) and (b) whether or not the filename was user-chosen (similar to how we handle aAfterFilePicker
). Then non-user-chosen filenames can be compromised in length based on the target location (if available, and MAX_PATH / 2
if not), and we can leave others alone.
I also confess that I am confused about what exactly Windows' restrictions are here. Why does it allow saving files that it then cannot deal with? Are the issues in comment 0 just other pieces of software that do not deal with extended filenames, given Windows Explorer is fine?
Comment 9•2 months ago
|
||
(In reply to :Gijs (he/him) from comment #8)
I also confess that I am confused about what exactly Windows' restrictions are here. Why does it allow saving files that it then cannot deal with? Are the issues in comment 0 just other pieces of software that do not deal with extended filenames, given Windows Explorer is fine?
Mostly, yes. MAX_PATH
is not a hard limit; I know of at least three ways around it. Some relevant documentation can be found here, here, and here.
Comment 10•2 months ago
|
||
(In reply to Ray Kraesig [:rkraesig] from comment #9)
I know of at least three ways around it.
It does not help unless File Explorer uses them. Quoting from the bug description:
If I try to double click on the URL to open the page in firefox, then Windows complains that can't/won't open it.
Also we can't force users to use the registry hack. It may be possible that we limit the filename length only if the registry setting is disabled.
Comment 11•2 months ago
|
||
We used to truncate the filename to 128 characters (see bug 250392). This commit effectively removed this limit.
Comment 12•2 months ago
|
||
Set release status flags based on info from the regressing bug 1746139
Updated•2 months ago
|
Comment 13•2 months ago
|
||
The simplest would be to just set the maximum filename length to something shorter on Windows, but passing a nsIFile target directory would also be an option.
Comment 14•2 months ago
|
||
but passing a nsIFile target directory would also be an option.
...except that it is impossible for Drag and Drop case. See comment #4.
Comment 15•2 months ago
|
||
:mkmelin, since you are the author of the regressor, bug 1746139, could you take a look?
For more information, please visit BugBot documentation.
Updated•2 months ago
|
Comment 16•1 month ago
|
||
(Not really my patch).
For Thunderbird we set the max length to 1/3 of maximum, https://phabricator.services.mozilla.com/D158825, and I haven't heard any complaints.
Comment 17•1 month ago
|
||
Set release status flags based on info from the regressing bug 1746139
Comment 18•1 month ago
|
||
The severity field is not set for this bug.
:Gijs, could you have a look please?
For more information, please visit BugBot documentation.
Comment 19•28 days ago
|
||
(In reply to Ray Kraesig [:rkraesig] from comment #7)
I have no reason to believe that the calls in
nsDataObj.cpp
are the only invocations of ValidateFileNameForSaving which could result in this issue.
I expect that in a lot of cases (normal downloads / "save page as", etc.) the filename will already have passed through a Windows file picker dialog and that (based on the earlier discussion) likely guarantees this can't be an issue as the picker presumably does not let you choose files that Windows Explorer doesn't know how to deal with.
Hardcoding a limit of 128 for all files (inside ValidateFileNameForSaving) would probably break some user workflows, so I don't think we should do only that. So I see 3 options:
- add the 128 limit back to the drag/drop code specifically
- add a 128 limit to ValidateFileNameForSaving with a flag to "escape" it, then audit all the callpaths and add this flag to any consumers where the leafname is already known to be below the MAX_PATH when combined with the destination (ie the filepicker case outlined above)
- add a 128 limit to ValidateFileNameForSaving and an ability to pass an
nsIFile
or full path, and only use the 128 limit if no full path / nsIFile is present. If more info is present, use that to limit the file name.
The second 2 options are probably "better", but also significantly more work.
Neil or Ray, I don't suppose you have cycles to work on this? If not, I can probably write a patch for (1) but don't have cycles to pursue 2 or 3 at the moment.
Comment 20•28 days ago
|
||
... either a filepicker, or we deal with things after moving to a final destination, cf.:
Comment 21•28 days ago
|
||
(In reply to :Gijs (he/him) from comment #19)
I expect that in a lot of cases (normal downloads / "save page as", etc.) the filename will already have passed through a Windows file picker dialog and that (based on the earlier discussion) likely guarantees this can't be an issue as the picker presumably does not let you choose files that Windows Explorer doesn't know how to deal with.
I'm not sure why you're presuming that, because the picker will absolutely let you do that. It has a couple of checks but they don't cover the whole possibility space.
Neil or Ray, I don't suppose you have cycles to work on this
I don't, sadly. Even if I did, I'd probably just go with 1) — I don't think 2) or 3) are worth the effort, especially given comment #16.
Updated•28 days ago
|
Comment 22•28 days ago
•
|
||
(In reply to Ray Kraesig [:rkraesig] from comment #21)
(In reply to :Gijs (he/him) from comment #19)
I expect that in a lot of cases (normal downloads / "save page as", etc.) the filename will already have passed through a Windows file picker dialog and that (based on the earlier discussion) likely guarantees this can't be an issue as the picker presumably does not let you choose files that Windows Explorer doesn't know how to deal with.
I'm not sure why you're presuming that, because the picker will absolutely let you do that. It has a couple of checks but they don't cover the whole possibility space.
Windows' own filepicker doesn't enforce MAX_PATH
and then produces unusable files? 😭
(The code I linked in comment 20 would still enforce stuff for temp files for downloads; I haven't looked to see if we have similar restrictions around the final file though I would naively expect us to... basically, by the law of numbers, and how many people drag-to-save vs download files, if this was broken for downloads in general, all the time, I'd have expected us to hear about it well before now. Ditto for "save page as".)
I don't, sadly. Even if I did, I'd probably just go with 1) — I don't think 2) or 3) are worth the effort, especially given comment #16.
OK, that's helpful input, thank you!
Description
•