User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

1 - Go to https://example.com/
2 - Open the console of the developer tools
3 - type the following JavaScript code: localStorage.getItem("test")

Actual results:

The following error is thrown:
[Exception... "File error: Not found" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 1" data: no]

Expected results:

No error should have been thrown. I see this regression on two different machines since the update to the version 67.0b3. It also breaks extension using the local storage API.

I cannot reproduce it on my Mac book pro with my Nightly (68.0a1 (2019-03-19) (64-bit)). (I got null)

FYI, resetting my profile fixed the issue.

Hm, it's probably too late, but you could try to run a debug build with original profile to get more information about the failure.

I still have the presumed defective profile. I'll try to do this test. Are there debug builds (already compiled) available for download?

You can go to https://treeherder.mozilla.org, look for mozilla-central repository.

For example:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=bdfe643caf38aae40683a8229aac211021813c03

Find your platform, for example: Windows 2012 x64 debug
Click on "B" and then click on Job Details
And finally click on target.zip

You need to run it from a command line to see debug output.

Thank you for the procedure. I was able to reproduce the issue with a debug build. Here is below what I see in the console when I run 'localStorage.getItem("test")'.

[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8256
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8301
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1810
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1865
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1951
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9745
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8464
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9026
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4365
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4527
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5070
[Parent 1416, QuotaManager IO] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5218
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5201
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/indexedDB/ActorsParent.cpp, line 19750
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/indexedDB/ActorsParent.cpp, line 19610
[Parent 1416, IPDL Background] WARNING: Converting non-IndexedDB error code (0x80520012) to NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR: file z:/build/build/src/dom/indexedDB/ActorsParent.cpp, line 559
JavaScript error: , line 0: uncaught exception: undefined
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8256
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8301
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1810
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1865
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1951
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9745
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8464
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9026
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4365
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4527
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5070
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/localstorage/ActorsParent.cpp, line 2794
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/localstorage/ActorsParent.cpp, line 6189
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/localstorage/ActorsParent.cpp, line 6628
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/localstorage/ActorsParent.cpp, line 5785

So this looks like a problem with upgrade from storage 2.1 to 2.2
For some reason metadata files can't be read or written.

Tom, can you take a look ?

Sure

(In reply to Gildas Lormeau from comment #7)
Below is my understanding:

[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8256
Fail to read the metadata file (Might because FILE_NOT_FOUND)

[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8301
Fail to read the metadata-v2 file (Might because FILE_NOT_FOUND)

[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1810
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1865
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1951
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9745
Fail to write a new metadata-v2 file

[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8464
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9026
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4365
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 4527
[Parent 1416, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5070
[Parent 1416, QuotaManager IO] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 5218
Fail to upgrade from 2_1 to 2_2

Gildas, the current information is too less for me to find the real cause. Would you mind providing me three more information? Thanks!
I guess I want:

• The name of the origin folder (e.g. https+++example.com)
• The error code of reading and writing the metadata file (e.g. 0x80520012)
• The permission of that folder.

To do that, would you mind running the debug build on [1]. It's the current code base with a patch for adding this information.
So steps to get this information would be:

• Go to a website to trigger storage initialization (e.g. do the STR you provide or go to "https://firefox-storage-test.glitch.me/")
• Check the debug messages and you should find messages like "Fail to read the metadata in the directory: xxx with reason ooo" or "Fail to create a metadata file in the directory: xxx with reason ooo".
  "xxx" should be the location of the problem origin folder and "ooo" should be the number of the error code in hex.
• Go to the location shown in "xxx" and check the permission of that folder.

Note that the message might leak the information about the website you have visited and the path of your profile. Please check whether you are fine with putting it on the Bugzilla. If you think it's a private thing, you could consider sending me an email if you don't mind.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ce3ccc51ae4f832f018794d331d1c0c43cf7048

I ran the test you described on https://example.com (because it produced less logs than https://firefox-storage-test.glitch.me/). I can see the following message in the console:

[Parent 4740, QuotaManager IO] WARNING: Fail to create a metadata file in the directory: C:\Users\Admin\AppData\Roaming\Mozilla\Firefox\Profiles\rgx3insk.dev-edition-default-1518558091672\storage\default\file++++E++T%C3%A9l%C3%A9chargements+Le%20Monde.fr%20-%20Actualit%C3%A9s%20et%20Infos%20en%20France%20et%20dans%20le%20monde(1).html with reason: 80520012: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9767

This directory can be accessed by 3 groups/users (system, myself, and admin) and each of them have all the permissions. Note that the file referred by this directory is a HTML file saved with the extension "Save Page WE" which uses Blobs to display embedded resources (images, frames). I noticed the extension never revoke them. Maybe that's related?

By the way, the directory is empty.

(In reply to Gildas Lormeau from comment #11)

Thanks for the help of debugging!

I ran the test you described on https://example.com (because it produced less logs than https://firefox-storage-test.glitch.me/). I can see the following message in the console:

[Parent 4740, QuotaManager IO] WARNING: Fail to create a metadata file in the directory: C:\Users\Admin\AppData\Roaming\Mozilla\Firefox\Profiles\rgx3insk.dev-edition-default-1518558091672\storage\default\file++++E++T%C3%A9l%C3%A9chargements+Le%20Monde.fr%20-%20Actualit%C3%A9s%20et%20Infos%20en%20France%20et%20dans%20le%20monde(1).html with reason: 80520012: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9767

So the error code is NS_ERROR_FILE_NOT_FOUND and it's a bit strange here because it's an error code for the function of creating data.

I manually create a directory with the same filename and I can still initialize successfully.

This directory can be accessed by 3 groups/users (system, myself, and admin) and each of them have all the permissions. Note that the file referred by this directory is a HTML file saved with the extension "Save Page WE" which uses Blobs to display embedded resources (images, frames). I noticed the extension never revoke them. Maybe that's related?

By means of all the permissions, could you elaborate that more? In my windows machine, I see a normal folder:
system, myself, and admin can: modify, read&execute, listfolder contens, read, write. And, they all don't have the special permissions.

You're welcome Tom. I confirm I see exactly the same permissions than you. All are active but not the special permissions.

(In reply to Gildas Lormeau from comment #14)

You're welcome Tom. I confirm I see exactly the same permissions than you. All are active but not the special permissions.

Gildas, could you also show me the message of "Fail to read the metadata ....". There should have one~two of them before you see "Fail to create a metadata file ...".

Tom, here are below the first "Fail to ..." messages I see in my logs.

[Parent 18856, QuotaManager IO] WARNING: Fail to read the metadata in the directory: C:\Users\Admin\AppData\Roaming\Mozilla\Firefox\Profiles\rgx3insk.dev-edition-default-1518558091672\storage\default\file++++E++T%C3%A9l%C3%A9chargements+Le%20Monde.fr%20-%20Actualit%C3%A9s%20et%20Infos%20en%20France%20et%20dans%20le%20monde(1).html with reason: 80520012.: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9723
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 2132
[Parent 18856, Main Thread] WARNING: '!workerClassifier', file z:/build/build/src/netwerk/url-classifier/AsyncUrlChannelClassifier.cpp, line 770
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 8301
[Parent 18856, QuotaManager IO] WARNING: Fail to read the metadata in the directory: C:\Users\Admin\AppData\Roaming\Mozilla\Firefox\Profiles\rgx3insk.dev-edition-default-1518558091672\storage\default\file++++E++T%C3%A9l%C3%A9chargements+Le%20Monde.fr%20-%20Actualit%C3%A9s%20et%20Infos%20en%20France%20et%20dans%20le%20monde(1).html with reason: 80520012.: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9737
++DOCSHELL 0000020FEA513000 == 4 [pid = 18856] [id = {dfe64fb5-6330-47a9-8ec6-be6a4e88b4e3}]
++DOMWINDOW == 8 (0000020FE8603200) [pid = 18856] [serial = 8] [outer = 0000000000000000]
++DOMWINDOW == 9 (0000020FEA630000) [pid = 18856] [serial = 9] [outer = 0000020FE8603200]
++DOMWINDOW == 10 (0000020FEA631800) [pid = 18856] [serial = 10] [outer = 0000020FE8603200]
[Parent 18856, Main Thread] WARNING: Failed to get base domain!: file z:/build/build/src/ipc/glue/BackgroundUtils.cpp, line 317
++DOMWINDOW == 11 (0000020FEA63A800) [pid = 18856] [serial = 11] [outer = 0000020FE8603200]
[Parent 18856, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805D0021: file z:/build/build/src/modules/libjar/nsJARChannel.cpp, line 994
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1810
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1865
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 1951
[Parent 18856, Main Thread] WARNING: Need TabChild to get the nativeWindow from!: file z:/build/build/src/widget/PuppetWidget.cpp, line 1078
[Parent 18856, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9760
[Parent 18856, Main Thread] WARNING: Need TabChild to get the nativeWindow from!: file z:/build/build/src/widget/PuppetWidget.cpp, line 1078
[Parent 18856, QuotaManager IO] WARNING: Fail to create a metadata file in the directory: C:\Users\Admin\AppData\Roaming\Mozilla\Firefox\Profiles\rgx3insk.dev-edition-default-1518558091672\storage\default\file++++E++T%C3%A9l%C3%A9chargements+Le%20Monde.fr%20-%20Actualit%C3%A9s%20et%20Infos%20en%20France%20et%20dans%20le%20monde(1).html with reason: 80520012: file z:/build/build/src/dom/quota/ActorsParent.cpp, line 9767

(In reply to Gildas Lormeau from comment #16)

Thanks! It's still confused that why would the error be the NS_ERROR_FILE_NOT_FOUND while creating the metadata file. I couldn't figure that by testing the code path in my Windows laptop. (I added an empty folder with that name and running upgrade logic, though)

Gildas, would you mind compressing that folder and attaching to here? I hope that could help me to find the issue behind.

(It would be also okay to send that to me privately if you wouldn't mind and think that's better).

Note that I guess you could fix the issue by removing the folder on your profile.

Tom, here is a link with the "default" parent directory and the folder: https://drive.google.com/open?id=1EOB-Bgu3dF4YAvqML6fFm1PaTLYcVB_1

I already fixed the issue by creating a new profile. However, I can try to delete the folder if you want.

(In reply to Gildas Lormeau from comment #18)

Tom, here is a link with the "default" parent directory and the folder: https://drive.google.com/open?id=1EOB-Bgu3dF4YAvqML6fFm1PaTLYcVB_1

I already fixed the issue by creating a new profile. However, I can try to delete the folder if you want.

Thanks! (I still cannot hit it by my mac after applying this to the test) Will test it with a Windows laptop on Monday.

I meant I know it's painful to give up the profile because it would lose all the record (persisted data). So, ideally, it'd be really nice for us if you can keep the broken environment until we fix that, but it's definitely fine for you to recover that. And, I suspect that you might have some issues else from the upgrade because the code for the upgrade has been applied when you reported the issue.

The original problem could be failing to restore the metadata during the initialization (InitializeOrigin() not the upgrade) because, in your broken folder, there is no metadata file.

Note to myself: I should try the extension "Save Page WE" on a Windows machine to see if I can reproduce the problem. I only tried to test that by writing tests and running files which are manually created in a debug build.

(In reply to Tom Tung [:tt, :ttung] from comment #19)

Note to myself: I should try the extension "Save Page WE" on a Windows machine to see if I can reproduce the problem. I only tried to test that by writing tests and running files which are manually created in a debug build.

I tested it on my Windows laptop and I still couldn't hit the issue (it initialized successfully).

I was wondering if this issue could be related to the length of the path. It looks like it's okay though. The article here https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#maximum-path-length-limitation says the max length is 260 characters by default. However, I noticed that on my machine the registry key "HKLM\SYSTEM\CurrentControlSet\Control\FileSystem LongPathsEnabled" is already set to 1, so I guess the max length is approximately 32,767 characters.

By the way, if I go to the default directory and try to copy the file+++ directory by copying and pasting it in the folder, Windows complains about the length of the path.

(In reply to Gildas Lormeau from comment #21)

I was wondering if this issue could be related to the length of the path. It looks like it's okay though. The article here https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file#maximum-path-length-limitation says the max length is 260 characters by default. However, I noticed that on my machine the registry key "HKLM\SYSTEM\CurrentControlSet\Control\FileSystem LongPathsEnabled" is already set to 1, so I guess the max length is approximately 32,767 characters.

Thanks! That's one thing which I haven't noticed. It seems that our code does handle the MAX_PATH and thus it should return a proper error code while failing (instead of returning NS_FILE_NOT_FOUND). However, I'll test that this afternoon on my Windows machine.

[1] https://searchfox.org/mozilla-central/search?q=MAX_PATH&case=false&regexp=false&path=xpcom%2Fio%2FnsLocalFileWin.cpp

(In reply to Tom Tung [:tt, :ttung] from comment #23)

Thanks! That's one thing which I haven't noticed. It seems that our code does handle the MAX_PATH and thus it should return a proper error code while failing (instead of returning NS_FILE_NOT_FOUND). However, I'll test that this afternoon on my Windows machine.

Thank comment #21 from Gildas, right now I can reproduce it with the same error (NS_FILE_NOT_FOUND).

I'm going to change the tile of this bug.

Great news! I'm glad I contributed to fix this issue :)

I guess it's clear to me to have a better error code when it fails due to this. However, I need more time to think about if there is a good way to handle this issue rather than just returning an error.

I have patches for this. Will update that tomorrow.

Depends on D27073

Depends on D27074

(In reply to Tom Tung [:tt, :ttung] from comment #29)

Created attachment 9057538 [details]
Bug 1536796 - P2 - Remove the directory if NS_ERROR_FILE_NAME_TOO_LONG is received while creaeting a file on Windows;

Depends on D27073

Even with this, we need to add a patch for telemetry modification (adding a label for initialization)
And file a new bug for quota client, I believe they should face a similar issue.

(In reply to Tom Tung [:tt, :ttung] from comment #28)

After looking into more, I found that I could create a file with 259 char for its path by calling nsIFile API. However, I fail to do the same thing on QuotaManager.

Both of them would call OpenFile() in nsLocalFileWin.cpp. The only difference is the argument.
Calling by nsIFile.create:
aOSFlags: 153
aMode: 438

Calling by QM:
aOSFlags: 42
aMode: 436

They would make the parameters for CreateFileW different:
Calling by nsIFile.create:
GENERIC_READ, CREATE_NEW

Calling by QM:
GENERIC_WRITE, CREATE_ALWAYS

After checking the document for CreateFileW, these flags seem reasonable. I wonder if it's an issue on our side or Microsoft. I will check more.

(In reply to Tom Tung [:tt, :ttung] from comment #32)

After checking the document for CreateFileW, these flags seem reasonable. I wonder if it's an issue on our side or Microsoft. I will check more.

Quote from the [1]:
If the CreateNamedPipe function was not successfully called on the server prior to this operation, a pipe will not exist and CreateFile will fail with ERROR_FILE_NOT_FOUND.

I also found that sometime I would just get this error while creating a file on Windows. In general, I guess this is a Windows issue. The only thing I can do here is to check the file path manually on QM so that we wouldn't try to create files again and again.

[1] https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-createfilew

This path will deny the creation of an origin direcotry if the its path after
appending any internal file will exceed the MAX_PATH limitation on Windows. As
for the exsiting directories which hacing this issue, they will be deleted in
when its internal files are restoring.

Depends on D28034

We will also need to check the path on QuotaClients. I am thinking of doing that on a follow-up bug.

Can this be somehow integrated into nsIFile instead ?

(In reply to Jan Varga [:janv] from comment #37)

Can this be somehow integrated into nsIFile instead?

Jan, could you elaborate this more? We will get either NS_ERROR_FILE_NAME_TOO_LONG or NS_ERROR_FILE_NOT_FOUND when the creating file has a path >= 260 char or creating directory has a path >= (260-12) char. So, my plan is to additionally check the component having an internal file which requires more than 12 char.

We could also check when the error comes out, but it probably means we need to check in all the places when we are creating a file.

If we are going to do that on nsIFile, I guess we need to provide the information about where should be removed as well if we don't want that to bother other files.
e.g.
The error comes out when creating below file
default/https+++example.com/.metadata-tmp
We need to tell nsIFile, "https+++example.com" should also be removed when the error occurs to avoid that blocking initialization.

I check the length before we actually get the error because the error will occur when QM creates the ".metadata-v2-tmp" file and we won't be able to recover from that unless we somehow avoid this by hashing the directory name (so that the path will be less) or some other ways. Anyway, I think this is still necessary because we allow users to create their own profile name and MAX_PATH might still happen.

Well, I think we have a bigger problem here. What if the CreateDirectoryMetadata2 call in RestoreDirectoryMetadata2Helper::ProcessOriginDirectory fails for some other reason ?
For example, creation of .metadafile-v2 succeeds, but writing to the file fails. What happens in that case ? Can we recover from that ? I think we should investigate that while we are there.

P1 currently tries to prevent a failure to not block temp storage initialization, but I think we should rather try to handle the case when ProcessOriginDirectory fails.

(In reply to Jan Varga [:janv] from comment #39)

Well, I think we have a bigger problem here. What if the CreateDirectoryMetadata2 call in RestoreDirectoryMetadata2Helper::ProcessOriginDirectory fails for some other reason ?
For example, creation of .metadafile-v2 succeeds, but writing to the file fails. What happens in that case ? Can we recover from that ? I think we should investigate that while we are there.

So do you mean we should also make ProcessOriginDirectory() better? If so, I will look into it to see what we can improve here.

Btw, in this case, from my understanding, we are passing PR_TRUNCATE and PR_WRONLY flag into the implementation of the nsIFile. Thus, the first round would fail, but the next try would be okay if the failure is temporary because they will be transferred to CREATE_ALWAYS which means the file will be overwritten even if it has created.

P1 currently tries to prevent a failure to not block temp storage initialization, but I think we should rather try to handle the case when ProcessOriginDirectory fails.

My impression of handling this is to remove the directory when it fails. I'll investigate that more.

(In reply to Tom Tung [:tt, :ttung] from comment #40)

So do you mean we should also make ProcessOriginDirectory() better? If so, I will look into it to see what we can improve here.

On the meeting, Jan confirm this. And, he also suggested me rather than checking metadata-only I should try to apply that to all the files if possible.

There is another opened issue (might be closed now) in indexedDB which is similar to this. It's related to File URI.

The issue appeared again in my profile, I deleted the directory "/storage/default/file++++E++T%C3%A9l%C3%A9chargements+El%20intercooler_%20qu%C3%A9%20es%20y%20c%C3%B3mo%20funciona%20(2019-06-28%201_38_09%20AM).html" and it fixed it immediately.

Any progress on this issue?

One of uBlock Origin users created issue on uBO issue tracker describing problems with uBlock Origin crashing and this causing Firefox to not being able to connect to Internet. This was triggered by saving and opening page saved by "Save Page WE" extension. This is caused by folder with very long name created in profiles/~/storage/default.

This looks to be caused by this bug.

Copy of issue from GitHub:

Description

Openning a saved html file by Save Page WE with a long name created an empty folder with the same name in profiles/~/storage/default, caused ublock Filter list to not load, and when I deletd the folder the settings are reset.
The name of empty folder was something like this (o replaced with some other charecters):
file++++C++Users+oooooooo+oooooooo+ooooo%20ooooooo+ooooooooooo+ooo%20oooo+MRAM%20vs%20SRAM%20vs%20DRAM-Difference%20between%20MRAM,SRAM%20and%20DRAM.html

A specific URL where the issue occurs

https://www.rfwireless-world.com/Terminology/MRAM-vs-SRAM-vs-DRAM.html

Steps to Reproduce

1. Go to https://www.rfwireless-world.com/Terminology/MRAM-vs-SRAM-vs-DRAM.html
2. Save webpage using Save Page WE
3. Open the saved webpage
4. The empty folder is created in profiles/~/storage/default
5. This folder make Firefox nonfunctional, crashing Ublock, etc
6. Removing the folder fixes the issue!

Your environment

• uBlock Origin version: v1.22.4
• Browser Name and version: Firefox Nightly 71.0a1
• Operating System and version: Windows 10

(In reply to gwarser from comment #43)

Any progress on this issue?

First of all, thanks for reporting this and providing information in detail!

However, unfortunately, I switched my context to another stuff. I will try to provide a temporary fix for this based on Jan's comment in comment 39 this week. ni myself to remind me of doing that

Note that I said the temporary fix because that probably won't make us be able to access that origin (which has the long file path). I would probably just remove the file to make the quota storage work. The follow-up/actual fix should be something like hashing the directory name into some shorter string so that we can avoid hitting this error and we can safely access that origin.

Bugbug thinks this bug is a regression, but please revert this change in case of error.

(In reply to Tom Tung [:tt, :ttung] from comment #44)
I cannot find a time slot to work on this. will see if I can find one next week

I'm also affected by this bug. See detailed information in duplicated bug #1589963. In short - very long folder names at default storage will breaks Mozilla Firefox usability in my case. In affected profile extensions, database and storage will be broken. Everything will be working extremely slow with very high CPU usage, extensions won't be working properly, website pages will be missing content like images and saved proprieties. What's more, disabling all extensions or even Safe Mode don't fix issue. Only manually deleting folders with very long name from default storage will revert everything "magically" to normal.

[Tracking Requested - why for this release]:
Storage can break Firefox profiles

I agree that this seems really critical. Did we have a regression range for this or is not a regression? Setting tracking flags to get more eyes on this and tracking a possible uplift.

Similar issues (copied from duped bug #1589963):

• bug #944918
• bug #1267506
• bug #1357294
• bug #1408141

IMO looks like long standing regression (if even).

Thanks!

Looks like it is concerning and has several duplicates, but also isn't a new regression in 70 and only affects users with particularly long folder names. Not a release blocker. We can aim to fix it in 71 though.
Johann, please let me know if you disagree and want to try to get this into a 70 dot release.

To be clear, Tom is the owner of this bug, I just wanted to get it on your radar :)

I don't think this can go in a 70 dot release, but tracking 71 would be nice, IMO. It would depend on what Tom thinks how quickly this can be done.

If someone have privacy.firstparty.isolate enabled then it's probable to hit this on long domains:

http+++www.thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com^firstPartyDomain=thelongestdomainnameintheworldandthensomeandthensomemoreandmore.com

This example is even longer than reported in uBO tracker (comment 43)

Wasn't this caught in telemetry from https://bugzilla.mozilla.org/show_bug.cgi?id=1529016 ? Hmmm, looks like it was inside if (!isDirectory).

My naive plan is:

• Have a bug for hashing origin folders with a long filename
• P1 should stop creating new problematic origin folders. After this patch, we can avoid creating folders which block storage initialization
• Have a minor/major upgrade to remove empty directories

Combine the last two items together we should avoid the filename too long issues on existing/future metadata files.
The first item should fix the actual issue.

(In reply to Tom Tung [:tt, :ttung] from comment #56)

• P1 should stop creating new problematic origin folders. After this patch, we can avoid creating folders which block storage initialization

I know this is not typical use case (saved web page on disk), but do you mean that storage APIs which depend on QuotaManager wouldn't work for in these cases ?

(In reply to Jan Varga [:janv] from comment #57)

(In reply to Tom Tung [:tt, :ttung] from comment #56)

• P1 should stop creating new problematic origin folders. After this patch, we can avoid creating folders which block storage initialization

I know this is not typical use case (saved web page on disk), but do you mean that storage APIs which depend on QuotaManager wouldn't work for in these cases ?

I meant, for instance, an origin has a long file path and the web page is using any quota client. In that case, if we happen to be able to create the origin directory but we cannot create the metadata file because of the filename too long issue. We won't leave an empty origin directories and keep blocking initialization in the future.

Ok, so that means the problematic origin won't be able to use storage APIs.

To elaborate more, my understanding of this issue is that when we are creating an origin directory. We assume we can create a metadata file and at least one client directory after that. If there is an error happen between these, we would keep failing for initialization (because we will restore metadata as an example).

To avoid this, my proposal is to fall back to the original save point (before we create the origin directory) rather than leaving an empty directory.
So we need to remove the origin directory when unsuccessfully create a metadata file in this case.

(In reply to Jan Varga [:janv] from comment #59)

Ok, so that means the problematic origin won't be able to use storage APIs.

Yes, that's why I said "avoid". At this moment, I think we need to map the origin name into some shorter name (and store the map on SQLite database and maybe metadata file). So I proposed item 1.

I see, that should be ok for now.

(In reply to Tom Tung [:tt, :ttung] from comment #60)

I think we need to map the origin name into some shorter name (and store the map on SQLite database and maybe metadata file)

Yes, UUID based origin directories, but this is more like long term solution.

We are working fixing other issues for storage initialization so this is probably won't happen on 71. Sorry about that.

The good news is that we will fix this in a better way (not just like the patch here, which only avoids blocking initialization) in QuotaManager v3.

Marking as wontfix for 71 as we have shipped our last beta + per comment #64

So I think we should focus on two places in the code right now:

1. GetDirectoryMetadata2WithRestore called from InitializeRepository
2. ProcessOriginDirectory called from StorageOperationBase::ProcessOriginDirectories

We don't want to delete entire origin directories when these calls fail without further checking what actually happened.
CreateDirectoryMetadata2 should be probably modified to let the caller know that it failed due to long file path, so the caller then can delete the directory.

There are other places where we enumerate origin directories and try to get metadata, but let's focus on those two places mentioned above.
We now have new telemetry for measuring storage initialization success rate (telemetry for upgrade methods is about to land too), so we can check if situation improved after these fixes.

If the path is still quite long (but not too long for creating the metadata file) we can fail somewhere in quota client directories which could cause initialization problems too in theory, but that's out of scope of this bug and we can try to address it later.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a092891cc9138e701e77cfa90331e44adc02d86c

https://treeherder.mozilla.org/#/jobs?repo=try&revision=274cbb39034908da76f2d14b1ce871022950ec6e

One more: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0aabf7a50da5480f9008bbb59a0043f5ec1f13d3

Patches here need to be rebased to the top of "ignore unkown directory"

Jens, can you find someone else to land this or do you think it should wait for Tom to get back?

Liz told me this needs to land sometimes in January for 74, so we can finish bug 1594075 first as planned and then we get back to this bug.

:lizzard, I assume, Jan's comment answers your question (indirectly)? Thanks!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bb0e82be840a799fe5193e6aa7b11e7ee70fd92c

I tried the UNC path approach by prepending that to mResolvedPath, but that it's not enough. It seems there is an issue on GetFileAttributesExW and a UNC file path exceeding the MAX_PATH. I cannot find similar cases by searching "GetFileAttributesExW", and "UNC file path", though.

Will keep working on it to see if I can fix the file name too long issue by adding the UNC prefix.

My patch roughly likes:

diff --git a/xpcom/io/nsLocalFileWin.cpp b/xpcom/io/nsLocalFileWin.cpp
--- a/xpcom/io/nsLocalFileWin.cpp
+++ b/xpcom/io/nsLocalFileWin.cpp
@@ -500,17 +514,17 @@ static void FileTimeToPRTime(const FILET
 #endif
 }
 
 // copied from nsprpub/pr/src/{io/prfile.c | md/windows/w95io.c} with some
 // changes : PR_GetFileInfo64, _PR_MD_GETFILEINFO64
 static nsresult GetFileInfo(const nsString& aName, PRFileInfo64* aInfo) {
   WIN32_FILE_ATTRIBUTE_DATA fileData;
 
-  if (aName.IsEmpty() || aName.FindCharInSet(u"?*") != kNotFound) {
+  if (aName.IsEmpty() /* || aName.FindCharInSet(u"?*") != kNotFound*/) {
     return NS_ERROR_INVALID_ARG;
   }
 
   if (!::GetFileAttributesExW(aName.get(), GetFileExInfoStandard, &fileData)) {
     return ConvertWinError(GetLastError());
   }
 
   if (fileData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) {
@@ -832,21 +846,21 @@ nsresult nsLocalFile::ResolveAndStat() {
 
   AUTO_PROFILER_LABEL("nsLocalFile::ResolveAndStat", OTHER);
   // we can't resolve/stat anything that isn't a valid NSPR addressable path
   if (mWorkingPath.IsEmpty()) {
     return NS_ERROR_FILE_INVALID_PATH;
   }
 
   // this is usually correct
-  mResolvedPath.Assign(mWorkingPath);
+  AssignResolvedPath();
 
   // Make sure root paths have a trailing slash.
-  nsAutoString nsprPath(mWorkingPath);
-  if (mWorkingPath.Length() == 2 && mWorkingPath.CharAt(1) == u':') {
+  nsAutoString nsprPath(mResolvedPath);
+  if (mResolvedPath.Length() == 2 && mResolvedPath.CharAt(1) == u':') {
     nsprPath.Append('\\');
   }
 
   // first we will see if the working path exists. If it doesn't then
   // there is nothing more that can be done
   nsresult rv = GetFileInfo(nsprPath, &mFileInfo64);
   if (NS_FAILED(rv)) {
     return rv;
@@ -862,17 +876,17 @@ nsresult nsLocalFile::ResolveAndStat() {
   }
 
   // we need to resolve this shortcut to what it points to, this will
   // set mResolvedPath. Even if it fails we need to have the resolved
   // path equal to working path for those functions that always use
   // the resolved path.
   rv = ResolveShortcut();
   if (NS_FAILED(rv)) {
-    mResolvedPath.Assign(mWorkingPath);
+    AssignResolvedPath();
     return rv;
   }
   mResolveDirty = false;
 
   // get the details of the resolved path
   rv = GetFileInfo(mResolvedPath, &mFileInfo64);
   if (NS_FAILED(rv)) {
     return rv;
@@ -896,17 +910,17 @@ nsresult nsLocalFile::Resolve() {
   }
 
   // we can't resolve/stat anything that isn't a valid NSPR addressable path
   if (mWorkingPath.IsEmpty()) {
     return NS_ERROR_FILE_INVALID_PATH;
   }
 
   // this is usually correct
-  mResolvedPath.Assign(mWorkingPath);
+  AssignResolvedPath();
 
   // if this isn't a shortcut file or we aren't following symlinks then
   // we're done.
   if (!mFollowSymlinks || !IsShortcutPath(mWorkingPath)) {
     mResolveDirty = false;
     return NS_OK;
   }
 
@@ -3270,16 +3284,27 @@ void nsLocalFile::EnsureShortPath() {
   // wide characters not including null termination is returned.
   if (lengthNeeded != 0 && lengthNeeded < ArrayLength(shortPath)) {
     mShortWorkingPath.Assign(shortPath);
   } else {
     mShortWorkingPath.Assign(mWorkingPath);
   }
 }
 
+void nsLocalFile::AssignResolvedPath() {
+  MOZ_ASSERT(!StringBeginsWith(mWorkingPath, NS_LITERAL_STRING("\\\\?\\")));
+  if (mWorkingPath.Length() >= (MAX_PATH - 12)) {
+    mResolvedPath.AssignLiteral("\\\\?\\");
+    mResolvedPath.Append(mWorkingPath);
+    return;
+  }
+
+  mResolvedPath.Assign(mWorkingPath);
+}
+
 NS_IMPL_ISUPPORTS_INHERITED(nsDriveEnumerator, nsSimpleEnumerator,
                             nsIDirectoryEnumerator)
 
 nsDriveEnumerator::nsDriveEnumerator() {}
 
 nsDriveEnumerator::~nsDriveEnumerator() {}
 
 nsresult nsDriveEnumerator::Init() {
diff --git a/xpcom/io/nsLocalFileWin.h b/xpcom/io/nsLocalFileWin.h
--- a/xpcom/io/nsLocalFileWin.h
+++ b/xpcom/io/nsLocalFileWin.h
@@ -83,16 +83,18 @@ class nsLocalFile final : public nsILoca
   }
 
   nsresult ResolveAndStat();
   nsresult Resolve();
   nsresult ResolveShortcut();
 
   void EnsureShortPath();
 
+  void AssignResolvedPath();
+
   nsresult CopyMove(nsIFile* aNewParentDir, const nsAString& aNewName,
                     uint32_t aOptions);
   nsresult CopySingleFile(nsIFile* aSource, nsIFile* aDest,
                           const nsAString& aNewName, uint32_t aOptions);
 
   nsresult SetModDate(int64_t aLastModifiedTime, const wchar_t* aFilePath);
   nsresult HasFileAttribute(DWORD aFileAttrib, bool* aResult);
   nsresult AppendInternal(const nsString& aNode, bool aMultipleComponents);

Testing that with a long file uri (its file path exceeds 260 characters), and the temporary directory metadata file could be created. However, it couldn't be renamed to a normal directory metadata file. I got ERROR_PATH_NOT_FOUND while renaming it. The error code was returned when calling ::GetFileAttributesExW in GetFileInfo.

To verify that this issue is not directly related to the way I add the UNC prefix, I tested that with a file uri which would have 259 characters for the temporary directory metadata file (so that the mResolvedPath would prepend with \\?\ by my current patch; note: it's because I can not differentiate the file type before getting the info so I assume file type is directory, for now, to avoid the shorter restriction on directories). The origin can be initialized and the indexedDB database can be generated.

Not sure if you have already been doing this (at least it's not part of the draft patch), but there are already tests for nsIFile in xpcom/tests/gtest/TestFile.cpp. Tests for long paths should be added there as well.

GetFileAttributesExW should work with \?\ according to:
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfileattributesexw

(In reply to Simon Giesecke [:sg] [he/him] from comment #76)

Not sure if you have already been doing this (at least it's not part of the draft patch), but there are already tests for nsIFile in xpcom/tests/gtest/TestFile.cpp. Tests for long paths should be added there as well.

That's my plan for the next step (if I succeed to use UNC prefix solution). Thanks for the reminder!

(In reply to Jan Varga [:janv] from comment #77)

GetFileAttributesExW should work with \?\ according to:
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfileattributesexw

Yeah, I saw that article, and that's the part I don't understand. I will figure out the reason why.

(In reply to Tom Tung [:tt, :ttung] from comment #75)

Testing that with a long file uri (its file path exceeds 260 characters), and the temporary directory metadata file could be created. However, it couldn't be renamed to a normal directory metadata file. I got ERROR_PATH_NOT_FOUND while renaming it. The error code was returned when calling ::GetFileAttributesExW in GetFileInfo.

I misread the log from Visual Studio debugger. There was an earlier error and had been generated at another place. It's because the filePath and destPath don't have a UNC prefix in my test (because they are appended and the parent node doesn't exceed the MAX_PATH limitation). Now, the simple test I created works. I should be able to prepare a patch tomorrow.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c48fe7a89f65128014591f70c9e2fa24c69693e
Things to do:

• Check the reason why I cannot just add "\?" prefix to all file paths. (I got some errors while doing that; Current patches only prepend that when it's necessary)
• Polish tests a bit
• File ends with dot
  • I still hit the assertion during getting quota objects for DOM Cache even when we can create/delete files/directories ending with dot by prepending "\?". There seem to have some other issues. (LSNG works fine with that, though)

(In reply to Tom Tung [:tt, :ttung] from comment #80)

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c48fe7a89f65128014591f70c9e2fa24c69693e
Things to do:

• Check the reason why I cannot just add "\?" prefix to all file paths. (I got some errors while doing that; Current patches only prepend that when it's necessary)

It's probably OpenFile has limited capabilities (e.g. doesn't support Unicode file name). I will try to change that to CreateFileW

• Polish tests a bit
• File ends with dot
  • I still hit the assertion during getting quota objects for DOM Cache even when we can create/delete files/directories ending with dot by prepending "\?". There seem to have some other issues. (LSNG works fine with that, though)

Note that functions like ILCreateFromPathW(), PathRemoveFileSpecW(), and
ExpandEnvironmentStringsW() don't support a file path exceeds the MAX_PATH even
with the "\?" prefix. Thus, there are still some functions in nsLocalFileWin
still having MAX_PATH restriction. However, most of them should work fine with
path being longer than MAX_PATH or ending with a dot now.

Depends on D60870

Depends on D60871

It turns out that I got some unreproducible test failures (e.g. GeckoMediaPlugins.RemoveAndDeleteForcedSimple) on my Windows machine if I applied the prefix to all files (https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=286327666&revision=034249f8dfb4132b063f56efa8290376d5fc8eb1).

Thus, I tried to minimalize the affected file path change. Here is the result.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9eebdde91635c9425dadce307fbf751ef95b149

Last try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48c75aef8198cb1bac1a9bf7434dfd3096d674bb&selectedJob=289929873

It looks not good, but it's because my patches cause tests to leak. I will fix that and hopefully can request review.

Wontfix for 74 as we are shipping our last beta.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aaa5eefbd1213485a215dc224e6fc61ddb40e5c7

(In reply to Tom Tung [:tt, :ttung] from comment #88)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aaa5eefbd1213485a215dc224e6fc61ddb40e5c7
and this one https://treeherder.mozilla.org/#/jobs?repo=try&revision=c92df9406c8f36e30b84c942dbe8bc9046c5e2e2
The patches try to do:

• Add the "\?" to every absolute file path (non-UNC) so that we don't need to check if the path is needed to add the prefix on each nsIFile::Append, nsIFile::CTOR
• Hide the prefix when nsIFile::GetPath because I don't want to let all the callers take care of the file path differently.

The issue is probably because there is another File implementation. During the marrionate server initialization, a file can be accessed by two different implementations (nsIFile and OS.File).

I haven't figured this out, but it seems that after accessing (or creating) a file with the prefix, accessing the file without the prefix would fail.

Thus, I change to the direction to:

• Add the prefix only when necessary
• Don't hide the prefix on the file path to make it possible to propagate around different implementations.

And here is the try for that: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e01ccdb8d3c6a09616529e2321ae0840ca0ecac

https://treeherder.mozilla.org/#/jobs?repo=try&revision=af15f2f71d09dae74d6606a068e3fcb2aa234430

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e27b6a9f084a4a118cfaef4314cd9d37108754c6

If this goes well, then I will polish patches and send them to be reviewed

(In reply to Tom Tung [:tt, :ttung] from comment #91)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e27b6a9f084a4a118cfaef4314cd9d37108754c6

If this goes well, then I will polish patches and send them to be reviewed

There is one more issue:
path:\?\Z:\task_1583848093\AppData\Roaming\Mozilla\Firefox\Profiles\6fdd4nrh.marionette-test-profile-1583851305186-1583851307086\storage\default\moz-extension+++649d8873-07b8-462c-834a-43276a1e4223^userContextId=4294967295\idb\3647222921wleabcEoxlt-eengsairo.sqlite

directoryPath:Z:\task_1583848093\AppData\Roaming\Mozilla\Firefox\Profiles\6fdd4nrh.marionette-test-profile-1583851305186-1583851307086\storage\default\moz-extension+++649d8873-07b8-462c-834a-43276a1e4223^userContextId=4294967295\idb

Assertion failure: StringBeginsWith(path, directoryPath), at z:/build/build/src/dom/quota/ActorsParent.cpp:4700

This patch is to prepend the "\\?\" prefix to absolute path on Windows when
it's needed.

That is only needed when:

• One of components ends with a dot. e.g. *\file.*
  (note: component like ".", "..", and "..." are not allowed for Windows path
  and are exculded from this)
• Total length of the file path exceeds the limitation on Windows (260 for
  files, and 248 for directories)

The reason for why not applying this to all Windows absolutes pathes:
To minimize unexpected side effects, this patch only prepend when it's
necessary.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae691b98fe1fe15d71823d14087d23a6906ece4d

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f90cd80b5a8f4d7bea4a6ebe2e3e1e993a8d3ab&selectedJob=292855109

This patch is to prepend the "\\?\" prefix to absolute path on Windows when
it's needed.

That is only needed when:

• One of components ends with a dot. e.g. *\file.* (note: component like ".",
  "..", and "..." are not allowed for Windows path and are excluded from this)
• Total length of the file path exceeds the limitation on Windows
  (260 for files, and 248 for directories)

The reason for why we don't applying this to all Windows pathes:
To minimize unexpected side effects, this patch only prepends the prefix when
it's necessary.

Now, Attachment #9133628 [details] handles this issue by prepending the \?\ prefix under certain conditions to all consumers of nsIFile on Windows. (So that we can restrict the effect.)

Jan just suggested that maybe we can:

• Apply \?\ prefix to all file paths on Windows
• Only do this under QuotaManager, and its clients (dom/indexedDB, dom/cache, dom/simpledb, dom/localstorage, and dom/quota)

Therefore, we can restrict the scope by only using it under QuotaManager and its clients.

To do that we probably need to:

• Add a new writable attribute to the nsIFile interface (for Windows)
• Add one more argument to the NS_NewLocalFile

Nathan, what do you think about this? Does it sound good to you? (If it's, I will abandon current patch and work on the new plan)

(In reply to Tom Tung [:tt, :ttung] from comment #97)

To do that we probably need to:

• Add a new writable attribute to the nsIFile interface (for Windows)
• Add one more argument to the NS_NewLocalFile

Nathan, what do you think about this? Does it sound good to you? (If it's, I will abandon current patch and work on the new plan)

I'm not against the writable attribute, I think -- though it's not clear to me what the attribute would represent -- but I don't think adding any more arguments to NS_NewLocalFile is a good idea. Can we do something else instead?

We would remove the IsLongFileNamePrefixNeeded method from the current patch, and add a new nsIFile writable attribute (set to false by default). Only QM and QM clients would set it to true. That way all other consumers wouldn't be affected at all and at the same time we would cover other problems that the prefix is supposed to fix on Windows (long file names and the trailing dot are only two cases we know about at the moment, but I read there are other pitfalls).

I thought that we could extend NS_NewLocalFile, but that is actually not essential for the new approach. We can create something like QM_NewLocalFile which will set the attribute after calling NS_NewLocalFile. QM_NewLocalFile would be used by QM and QM clients.

(In reply to Jan Varga [:janv] from comment #99)

I thought that we could extend NS_NewLocalFile, but that is actually not essential for the new approach. We can create something like QM_NewLocalFile which will set the attribute after calling NS_NewLocalFile. QM_NewLocalFile would be used by QM and QM clients.

Discussed this with :froydnj on Slack, and he prefers Jan's approach if it would work out.

I will work on this approach.

Per https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file section "Win32 File Namespaces"
the \? prefix disables all string parsing, so the nsIFile attribute could be named disableStringParsing.

They also mention ".." besides ".", so we can cover problems with ".." too.

Lastly, "Many but not all file I/O APIs support \?".
If we enable this only for QM, we can avoid potential problems with other consumers.

Comment on attachment 9133628 [details]
Bug 1536796 - P2 - Handle disable string paring in nsLocalFileWin;

Revision D67014 was moved to bug 1536795. Setting attachment 9133628 [details] to obsolete.

Depends on D67014

This test tries to remove profile by rmtree. However, since the "\?" prefix
has been applyied to QM and its clients on Windows and thus they are able to
create files which exceed the MAX_PATH limitation.

Therefore, we need to prepend the prefix to the remove function in this test as
well.

Depends on D67875

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b8d45ad8a486f3765d126f21bd66c1e4687b570&selectedJob=294554590

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8fad51766eae6c99681c108427f22174d031218&selectedJob=295047282

This test tries to remove profile by rmtree. However, since the "\?" prefix
has been applied to QM and its clients on Windows and thus they are able to
create files which exceed the MAX_PATH limitation.

Therefore, we need to prepend the prefix to the remove function in this test as
well.

Note that the problem was caused by an extension that uses IDB which was not
able to create a directory for stored files. The test doesn't check IDB so it
always passed before, despite the IDB failure. However, preceding patches make
it possible to use long file names in QM and QM clients, so IDB implementation
is now able to create the directory and the test needs to be able to delete it
as well.

Depends on D67876

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7171d824119844d48d376fc05389aef16a323730

There were some failures caused by P4b and hopefully I can fix them on this try. If not, I will file a followup bug for apply \\?\ to mozfile.remove and rollback the changes on P4.

(In reply to Tom Tung [:tt, :ttung] from comment #110)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7171d824119844d48d376fc05389aef16a323730

There were some failures caused by P4b and hopefully I can fix them on this try. If not, I will file a followup bug for apply \\?\ to mozfile.remove and rollback the changes on P4.

Oops, I miss one place so that test_refresh_profile.py failed again

here is try after one more fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=19dd2dc9d0d4ff6654536c7983d7ae87583b1888

Comment on attachment 9137248 [details]
Bug 1536796 - P4b - Use the prefix for mozfile.remove;

Revision D69086 was moved to bug 1626581. Setting attachment 9137248 [details] to obsolete.

I decided to give up the changes on mozfile.remove and plan to deal with that on a followup bug.

try for now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9cb099c82e80590fc63cf19044461d5e214b827

https://hg.mozilla.org/mozilla-central/rev/a8de4bf44281
https://hg.mozilla.org/mozilla-central/rev/34bedcaa4ade
https://hg.mozilla.org/mozilla-central/rev/be05a102d04c
https://hg.mozilla.org/mozilla-central/rev/906c9edbf623
https://hg.mozilla.org/mozilla-central/rev/0f7625476fcf
https://hg.mozilla.org/mozilla-central/rev/990288d76a16

Not sure if I should post here or create a new bug?

Following the most recent update to Nightly - Build ID 20200401212659 - storage seems to be completely broken. All addons have stopped working.

I ran a mozregression and it pointed to this bug.

Pushlog

2020-04-02T13:25:32: INFO : application_version: 76.0a1
2020-04-02T13:25:32: INFO : platform_buildid: 20200401122401
2020-04-02T13:25:32: INFO : platform_changeset: 876a51b67aca75334dbe942e425d4eab26919b6b
2020-04-02T13:25:32: INFO : platform_repository: https://hg.mozilla.org/integration/autoland
2020-04-02T13:25:32: INFO : platform_version: 76.0a1
2020-04-02T13:25:41: INFO : [Parent 2028, Gecko_IOThread] WARNING: file /builds/worker/checkouts/gecko/ipc/chromium/src/base/process_util_win.cc, line 166
2020-04-02T13:25:50: INFO : Narrowed integration regression window from [39c389ff, 990288d7] (3 builds) to [876a51b6, 990288d7] (2 builds) (~1 steps left)
2020-04-02T13:25:50: DEBUG : Starting merge handling...
2020-04-02T13:25:50: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=990288d76a16f51a970b512ef9d97990205b312d&full=1
2020-04-02T13:25:52: DEBUG : Found commit message:
Bug 1536796 - P6 - Changes on dom/quota unit test to verify the fix; r=dom-workers-and-storage-reviewers,janv

Differential Revision: https://phabricator.services.mozilla.com/D60872

2020-04-02T13:25:52: DEBUG : Did not find a branch, checking all integration branches
2020-04-02T13:25:52: INFO : The bisection is done.
2020-04-02T13:25:52: INFO : Stopped

Tom, can you quickly file a new bug and provide a patch that disables the DOS file path syntax ?

Sorry for the inconvenience! I filed bug 1626846 and updated a patch. Will send it to be reviewed after try is green.

Here is documentation that was used for implementation of the prefix:
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamea
https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats

One additional note, the prefix enables long file paths, but a path component is still limited to 255 chars. So if there's an origin longer than that, we won't be able to store stuff for it.
However, temporary storage intialization shouldn't fail because of that (someone needs to verify it), it can only fail when useDOSDevicePathSyntax is false and a long origin was already created when useDOSDevicePathSyntax was true.

Are you aware that many people had issue with broken add-ons after update to 77? For example https://www.reddit.com/r/firefox/comments/gvc7zd/none_of_my_extensions_work_now_with_firefox_77/

They said that setting network.file.disable_unc_paths = false in about:config worked for them.

Is this bug actually fixed? Do setting above pref have any bad side effects?

(In reply to gwarser from comment #121)

Are you aware that many people had issue with broken add-ons after update to 77? For example https://www.reddit.com/r/firefox/comments/gvc7zd/none_of_my_extensions_work_now_with_firefox_77/

They said that setting network.file.disable_unc_paths = false in about:config worked for them.

Is this bug actually fixed? Do setting above pref have any bad side effects?

Hi,

Thanks for reporting and noticing!

To be honest, I wasn't aware that many people had issues with network.file.disable_unc_paths because only one person reported this issue (comment#116). But, we have resolved this in FF78. That issue should be fixed in https://bugzilla.mozilla.org/show_bug.cgi?id=1634267.

Another way to disable functions that implemented in this ticket in FF77 is to set dom.quotaManager.useDOSDevicePathSyntax = false.

Edit:

Is this bug actually fixed? Do setting above pref have any bad side effects?

The add-ons issue should be fixed in FF78. If not, please let me know or file a bug for that. And we will try to fix that soon.

Before FF78, either dom.quotaManager.useDOSDevicePathSyntax to false or network.file.disable_unc_paths to false can avoid the issue.

Setting dom.quotaManager.useDOSDevicePathSyntax to false would disable the patches here and thus fix the broken add-ons. The patches here fix the file name too long issue on Windows by using the \\?\ prefix.

And network.file.disable_unc_paths is implemented in Bug 1413868 and it avoids bypassing the proxy setting checks by typing UNC file path in the URL bar. Note that the user must type or paste the text. Webpages along cannot trigger that issue.

In general, I think it should be fine to set network.file.disable_unc_paths to false for avoiding the add-ons issue since the pref and the fix in Bug 1413868 is disabled by default. However, if you still have concerns about that, you can set dom.quotaManager.useDOSDevicePathSyntax to false in FF77 and flip it to true in FF78.

Ok, thanks. Somehow it turns out my message in above Reddit thread recommended to set both prefs, and original poster said network.file.disable_unc_paths = false is enough for him.

Also this commenter https://www.reddit.com/r/firefox/comments/gvc7zd/none_of_my_extensions_work_now_with_firefox_77/fspxmd9/ said:

Thank you. I follow ghacks very closely and unc paths confit did the trick for me.

This is the ghacks thread: https://github.com/ghacksuserjs/ghacks-user.js/issues/923

But there https://www.reddit.com/r/uBlockOrigin/comments/gyadhc/dashboard_broken_seeing_ads_troubleshooting_steps/ OP said he need other pref:

Setting network.file.disable_unc_paths = false on its own didn't help, but dom.quotaManager.useDOSDevicePathSyntax = false on its own did clear those errors up and make uBO work again.

I think these are the only messages I have seen with feedback other than "Thanks"/"Works".

(In reply to gwarser from comment #123)

But there https://www.reddit.com/r/uBlockOrigin/comments/gyadhc/dashboard_broken_seeing_ads_troubleshooting_steps/ OP said he need other pref:

Setting network.file.disable_unc_paths = false on its own didn't help, but dom.quotaManager.useDOSDevicePathSyntax = false on its own did clear those errors up and make uBO work again.

I think these are the only messages I have seen with feedback other than "Thanks"/"Works".

Hmm, that looks like another issue. I will check that and it would be useful if we can get the error log when failing.

I cannot reproduce uBO with dom.quotaManager.useDOSDevicePathSyntax = true and network.file.disable_unc_paths = false with an new profile in FF 77 on my Windows machine (WIn 10). I wonder if there are some other factors involved.

Hi, drjeats from reddit here.

I had saved part of the browser log, but I can still produce the error by reverting my about:config change. While signing up for bugzilla, I also noticed that my Gmail wouldn't load without useDOSDevicePathSyntax=false. I had a similar issue with reddit, but fixed it by clearing cookies.

Here are the errors that showup when I restart FF 77.0.1 fresh after reverting my about:config change (before navigating beyond the new tab page):

Error: Can't find profile directory. XULStore.jsm:66:15
a11y.sitezoom - Unknown scalar.
IndexedDB UnknownErr: ActorsParent.cpp:570
UnknownError: The operation failed for reasons unrelated to the database itself and not covered by any other error code. 2 ExtensionStorageIDB.jsm:831
    normalizeStorageError resource://gre/modules/ExtensionStorageIDB.jsm:831
    method chrome://extensions/content/child/ext-storage.js:273
    AsyncFunctionThrow self-hosted:697
IndexedDB UnknownErr: ActorsParent.cpp:570
UnknownError: The operation failed for reasons unrelated to the database itself and not covered by any other error code. 9 ExtensionStorageIDB.jsm:831
    normalizeStorageError resource://gre/modules/ExtensionStorageIDB.jsm:831
    method chrome://extensions/content/child/ext-storage.js:273
    AsyncFunctionThrow self-hosted:697
IndexedDB UnknownErr: ActorsParent.cpp:570
UnknownError: The operation failed for reasons unrelated to the database itself and not covered by any other error code. 4 ExtensionStorageIDB.jsm:831
    normalizeStorageError resource://gre/modules/ExtensionStorageIDB.jsm:831
    method chrome://extensions/content/child/ext-storage.js:273
    AsyncFunctionThrow self-hosted:697
Error: Can't find profile directory. XULStore.jsm:66:15
Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift” id=“key_browserToolbox” browser.xhtml
this.window.gBrowserInit is undefined ext-browser.js:1130
    get activeTab chrome://browser/content/parent/ext-browser.js:1130
    candidates chrome://extensions/content/parent/ext-tabs-base.js:1969
    InterpretGeneratorResume self-hosted:1151
    next self-hosted:1099
    query chrome://extensions/content/parent/ext-tabs-base.js:1988
    next self-hosted:1099
    from self-hosted:505
    query chrome://browser/content/parent/ext-tabs.js:948
    query self-hosted:844
    result resource://gre/modules/ExtensionParent.jsm:988
    withPendingBrowser resource://gre/modules/ExtensionParent.jsm:594
    result resource://gre/modules/ExtensionParent.jsm:988
    callAndLog resource://gre/modules/ExtensionParent.jsm:950
    recvAPICall resource://gre/modules/ExtensionParent.jsm:987
    AsyncFunctionNext self-hosted:693

(In reply to drjeats from comment #126)
Thanks for sharing your log from the browser console! However, we still need more information to identify the issue.

I also noticed that my Gmail wouldn't load

It's probably because your QuotaManager is broken and Gmail uses service worker which requires DOM Cache (that is managed by QuotaManager).

Could you:

1. Run the same stuff [1] with a new profile and help me to check if you still have the same issue?
   This would help us to identify if the issue is related to something in your operating system setting or not.
   You can fnd a bottom for creating a new profile in about:profiles. And then run a Firefox with that new profile (with dom.quotaManager.useDOSDevicePathSyntax = true)
   See more information of this in mdn

2. Run the same stuff [1] with a debug release build and share the log here.
   This would help us to know which function causes the issue or starts to fails.

• Note that before doing this step, please make sure that you make a copy of your profile and store it somewhere else.
• You can find the directory of your profile in about:profiles
• You can find a debug build for Release at https://treeherder.mozilla.org/#/jobs?repo=mozilla-release.
  • For instance, this is a release debug build for Windows x64: https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&selectedTaskRun=FS1NF5LdTt693c3YnPi5nw-0.
• After browsing into the link, click the "Job details" tab in the bottom panel and you can find "artifact uploaded: target.zip".
• Download the zip file ("target.zip"), unzip it, and run the firefox inside.
• Doing the same steps that you did again, you should be able to see logs like:
  0:03.16 pid:16429 [16429, QuotaManager IO] WARNING: 'NS_FAILED(rv)', file /home/shes050117/Work/mozilla-central/dom/quota/ActorsParent.cpp, line 2717
• Share the log here.

We might need several run trips for doing step 2 to analyze the issue. Sorry for the inconvenience and thank you in advance!
An alternative for step 2 here is that, if you don't really want to do that in several times and trust us enough, you can send me your zipped profile via email.

Edit:
[1] Same stuff here I meant is:

• Ensure dom.quotaManager.useDOSDevicePathSyntax = true and network.file.disable_unc_paths = false
• Restart Firefox so that these prefs can be evaluated
• Check the uBO or go to "https://firefox-storage-test.glitch.me/"

Might be related. Immediate fix is a new profile.
Debug build for new profile: Storage works
Debug build for existing profile: Storage broken

(In reply to SJqhjXc4 from comment #128)

Might be related. Immediate fix is a new profile.
Debug build for new profile: Storage works

Thanks! That's really helpful!

I assume you set dom.quotaManager.useDOSDevicePathSyntax = true and network.file.disable_unc_paths = false

Debug build for existing profile: Storage broken

So, the first storage-related failure occurred when getting the padding size from the padding file.

I will first check if the file path for that file use the prefix (\\?\) or not.

(In reply to Tom Tung [:tt, :ttung] from comment #129)

(In reply to SJqhjXc4 from comment #128)
I will first check if the file path for that file use the prefix (\\?\) or not.

It uses the prefix (\\?\) as expected. (since the base directory comes from https://searchfox.org/mozilla-release/source/dom/cache/QuotaClient.cpp#353)

It would be interesting to see if the fresh profile would have the same issue on the storage of that website or not.
The issue happened while traversing the storage, thus the difference can be because of that. (The old one have the file/file path and the new one doesn't)

(In reply to Tom Tung [:tt, :ttung] from comment #129)

I assume you set dom.quotaManager.useDOSDevicePathSyntax = true and network.file.disable_unc_paths = false

Yes, both set as mentioned. Someone else discussed the issue, that's what lead me here: https://reddit.com/r/firefox/comments/h7smzt/all_my_extensions_stopped_working/

Both were tested with the below. Let me know if any more info is required.

firefox.exe -url https://firefox-storage-test.glitch.me/ > %UserProfile%\Desktop\firefox-new-profile.log 2>&1

firefox.exe -profile "EXISTING PROFILE" -url https://firefox-storage-test.glitch.me/ > %UserProfile%\Desktop\firefox-old-profile.log 2>&1

(In reply to SJqhjXc4 from comment #131)

Both were tested with the below. Let me know if any more info is required.

firefox.exe -url https://firefox-storage-test.glitch.me/ > %UserProfile%\Desktop\firefox-new-profile.log 2>&1

firefox.exe -profile "EXISTING PROFILE" -url https://firefox-storage-test.glitch.me/ > %UserProfile%\Desktop\firefox-old-profile.log 2>&1

Thanks! That would accelerate the process for us to find the root cause!

The website that causes your storage broken can be some other website rather than https://firefox-storage-test.glitch.me/, so I am preparing a custom build and will ask you to run it when it's ready.

In case you want to know some more information, I put more details below:
In theory, the first website that uses storage would trigger the profile traversing. And, from the log you provided, we know that storage for a website is broken, but we don't know which website it is. In this case, we only know that https://firefox-storage-test.glitch.me/ is the website that triggers the traverse.

I believe once we know the website's name, we can easily reproduce the issue with the website locally and thus figure the root cause.

So, I am building a custom build to dump the file path of the padding file while the access fails (path: https://hg.mozilla.org/try/rev/7b4484cf61faa664002f0c02b9d697e885dfce19).

The custom build on the treeherder is here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2978156596abf2b7b5e1b02f462c05edd07c2e6b.

I will needinfo you once it's ready. Many thanks!

(In reply to Tom Tung [:tt, :ttung] from comment #132)

I will needinfo you once it's ready. Many thanks!

It's ready now. Would you mind helping me to get more information by running the custom below and share it here or send me an email for the log if you don't want to share that here? Thanks in advance!

Here is a custom debug build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2978156596abf2b7b5e1b02f462c05edd07c2e6b&selectedTaskRun=TBVJk7IiTPGnqLyLCbD93A.0

Here is the build but a opt version: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2978156596abf2b7b5e1b02f462c05edd07c2e6b&selectedTaskRun=GCUjK1eRSg-ZVfiadK-Lhw.0

If you are okay to use the debug build, please run the same instruction with the old profile (firefox.exe -profile "EXISTING PROFILE" -url https://firefox-storage-test.glitch.me/ > %UserProfile%\Desktop\firefox-old-profile.log 2>&1)

If not, you can also try the link for opt version, you should find related log in your browser console like:
WARNING: Failed to get padding size for path (${file_path}): file /home/shes050117/Work/mozilla-central/dom/cache/FileUtils.cpp, line xxx.

(In reply to Tom Tung [:tt, :ttung] from comment #133)

I got emails for the logs from SJqhjXc4, thank you! The issue here is because of an edge case in DOM Cache. I will fill a ticket for that. (There seems to have another issue around directorylockId, but I will file a ticket once I get more information around it)

Restricting comments on this old fixed bug to cut down on spam.