Thunderbird handles folder names which are substrings of other IMAP namespaces incorrectly
Categories
(MailNews Core :: Networking: IMAP, defect)
Tracking
(Not tracked)
People
(Reporter: brong, Assigned: gds)
References
Details
Attachments
(1 file)
|
1.14 KB,
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0
Steps to reproduce:
Created a folder called "Other2" and tried to rename it to "Other" in the Thunderbird web interface. I expected it to create the folder "Other" on the server.
The server has the following namespaces:
3 namespace
- NAMESPACE (("" "/")) (("Other Users/" "/")) (("Other Folders/" "/"))
Actual results:
The command sent by Thunderbird to the server was:
17 rename "Other2" "Other Folders/Other"
Expected results:
It should have been renamed to "Other"!
The server rejected the rename into "Other Folders", since that namespace only allows admins to create top-level entries.
|
Assignee | |
Comment 1•7 years ago
|
||
(In reply to Bron Gondwana from comment #0)
Created a folder called "Other2" and tried to rename it to "Other" in the Thunderbird web interface.
I'm sure you mean "tb user interface" and not web interface. Don't know of a web interface for tb.
Also, I assume the folder "Other2" is in the Personal namespace and not in the Other Users or Shared/Public space?
Anyhow, I will try to duplicate this problem when I'm on my test system.
|
Reporter | |
Comment 2•7 years ago
|
||
Yes sorry! Thunderbird user interface. And yes, "Other2" is in the Personal namespace.
The backend is a Cyrus IMAP server running altnamespace: true and unixhierarchysep: true. You can easily test this against Fastmail if you have an account on our servers (e.g. a thunderbird.net account).
I haven't had an opportunity to look at the source code to see if I can see how this is happening, but it came in as a user report and I was able to easily reproduce it on my own account.
|
|
Assignee | |
Comment 3•7 years ago
|
||
I don't have a thunderbird.net account. Do you know how I can get one? (Or maybe Jorg or Wayne knows.)
Anyhow, on my test setup using a local zimbra server I have the "Other Users" namespace as "/home/". I was able to rename a folder in pers. namespace to homer but not to /homer since / is not allowed.
Looking at the code, I see that rename does something with namespace but I haven't yet determined what. New name probably has to start with a namespace string before there is a problem.
|
|
Reporter | |
Comment 4•7 years ago
|
||
I have emailed you directly a login and password that you can use for a month for debugging. Let me know if you need it extended beyond that.
|
|
Assignee | |
Comment 5•7 years ago
|
||
Thanks. The account worked and I see the same problem.
|
|
Assignee | |
Comment 6•7 years ago
|
||
Not sure but I think this may be describing a similar problem: Bug 902509. Or it has the same root cause.
If I create a folder called "Oth" with a non-tb client (that ignores or handles namespaces differently) and then try to delete it with tb, tb tries to delete the folder "Other Folders/Oth" and pops up an error that it doesn't exist, which is true.
I can't create the folder "Oth" in tb without first clearing users and shared namespace strings and disabling server override of namespaces in advanced server settings.
I found the code in tb that does the "prepend of the matching namespace" when the leading chars in the folder name match a namespace but don't know why this is even done. I haven't found any documentation on why this occurs. But if I comment out the "else" block that does this "prepend" this bug (feature?) goes away and folder with names like "Ot", "Othe" or "Other" can be created, deleted or renamed to in the default personal namespace just fine.
Not sure what happens when you rename, delete, or create a folder in the Other Users or Other Folders space since I don't see a folder in those spaces I can currently subscribe.
Bron, would that be possible to setup on your server with create/delete privileges in a user space so I can try it?
|
|
Assignee | |
Comment 7•7 years ago
|
||
I do have a local dovecot server with dual personal namespaces that I set up for other testing: namespaces INBOX and a-bear. I can probably do enough testing with it to see what's happening.
If I create folder a-b from the account level, it goes to to a-bear/a-b.
If I create folder c-b from the account level, it does to INBOX/c-b
If I create folder a-b from INBOX or a-bear, it goes to INBOX/a-b or a-bear/a-b respectively.
I can delete all the new folders under a-bear but for INBOX/a-b. Tb does this:
. rename INBOX/a-b INBOX/Trash/a-b
but the server returns an error for this: "Renaming not supported across conflicting directory permissions." Maybe its a dovecot setup error on my part.I can only delete the new folders under Inbox if I set tb "delete model" to mark as deleted. Then it just does imap DELETE and not RENAME into Trash.
So, other than the delete problem, this seems consistent with the observations noted by WADA in Bug 902509. Not sure if this is a bug or a feature...???
|
|
Reporter | |
Comment 8•7 years ago
|
||
So maybe the "account level" thing is that Fastmail sets "" as the prefix for the personal namespace. This is because more servers seem to be doing things that way rather than having INBOX as a root folder, so we switched behaviour a few years ago.
No other client has had an issue with it, and Thunderbird only has an issue if you create a folder with a name which is a substring prefix - I'm pretty sure that behaviour is totally bogus, but maybe somebody had a reason for it once?
Bron.
|
|
Reporter | |
Comment 9•7 years ago
|
||
I've had a look at that other bug - and the described workarounds in that bug don't work. The underlying cause does seem correct though:
[...] phenomenon you saw may be "comparison with NAMESPACE terminates at a space in NAMESPACE".
If folder name=="Shared", it's same as "Shared Folders" for Tb.
If folder name=="Others", it's same as "Other Users" for Tb.
If folder name=="Dossiers", it's same as "Dossiers Partagés" for Tb.
If comparison with NAMESPACE does indeed terminate a space, that would be a Tb bug.
|
|
Assignee | |
Comment 10•7 years ago
|
||
I don't think the tb compare terminates on a space in the namespace string. On the fastmail test account I just tried to create folder "Other F" and tb tried to create folder "Other Folders/Other F" which of course failed. In the code I can comment out of tb that does the prepend of the matching namespace string, I see nothing in it that treats the space char a special.
I think this tb just trying to helpful in ASSuming a leading namespace but I breaks when you create, rename or delete a folder that matches the leading chars of the namespace.
Yes, the bug you reported and others I see here don't exist at least in Evolution. E.g., I can create the folder "Other F" is evo but when I try to delete it with tb it say "no can do" because it tries to delete "Other Folders/Other F".
I will test some more with the "very helpful" prepending code commented out and see what I happens.
|
|
Assignee | |
Comment 11•7 years ago
|
||
I don't immediately see any big problem when I comment out lines 788-797:
https://searchfox.org/comm-central/rev/fd068a83dde907b700883313682899dc27aad874/mailnews/imap/src/nsImapUrl.cpp#788
https://searchfox.org/comm-central/rev/fd068a83dde907b700883313682899dc27aad874/mailnews/imap/src/nsImapUrl.cpp#797
(Note: The } on line 787 should be moved right by 1 space.)
If I create a folder from the account level, I have to manually type in the desired name space, e.g., Inbox/new-mbox or a-bear/new-mbox.
If I am selected on the namespace, e.g., a-bear or INBOX, I just enter the folder name, i.e., new-mbox. No problem deleting folders unless the server complains about moving to Trash same as before.
A problem is 99% of tb users probably don't have anything but one personal namespace so this doesn't get a lot of usage.
There is one other tb feature that this might affect that I tested. It is the advanced server setting "IMAP server directory". It is most commonly used to "flatten" the [Gmail] non-selectable folder so that all the gmail folders appear at root level under account and the gray [Gmail] folder isn't seen (set IMAP server dir to [Gmail]). I can also flatten and remove my "a-bear" namespace so it does not show and its subfolders show at the same level as Inbox (set server dir to a-bear). Folders created from the account always end up under a-bear, even if the name typed in is Inbox/nf (you end up creating a-bear/Inbox/nf). You have to be clicked on Inbox namespace to create a folder under the namespace Inbox.
So this change does cause a small change in functionality in that folders created from the top level (account) don't work exactly the same way as before. But I don't see it as being anywhere near as broken as what is reported in this bug. But some users may complain.
|
|
Assignee | |
Comment 12•7 years ago
|
||
My idea to just comment out some lines didn't really work when I did some more testing. This patch seems to work better. This prevents, in your case, access to root level folder "Other" from being sent to the server as "Other Folders/Other", where "Other Folders" is a namespace prefix. (This access occurs when the folder is renamed, created, deleted and selected.)
With this change I don't see any functionality changes (other than fixing the reported bug).
I have tested this quit a bit with dovecot and zimbra both with "other user" shared folders or multiple personal namespaces and don't see a problem. However, I could be missing something.
Bron, is it possible that you or one of your users might be willing test a "try" build that would essentially apply this patch to the latest 60.x? If so, let me know.
|
|
Assignee | |
Comment 13•7 years ago
|
||
I went ahead and did a try build for linux, win and macosx. Here are the direct links to the installation files:
I only tested the linux one and I'm never 100% sure which file is used to install on win and mac, so, if I'm wrong above, here's the link to the try build main page and you can pick the right file:
Click on the green "B" next to the line for the arch and along the bottom you will see more stuff. Then select "Job Details" and select the file to download from the list that appears.
This try build applies my patch to release 60.5.3. However, "About tb" will display as "Daily 60.5.3".
|
||
Comment 14•6 years ago
|
||
Bron, did you have success with the try builds?
|
|
Reporter | |
Comment 15•6 years ago
|
||
I never heard back from the user who I sent them to to confirm they were happy, but from memory the linux64 build worked for me at the time.
Bron.
|
|
||
Updated•6 years ago
|
|
||
Comment 16•6 years ago
|
||
I'm having the same problem using TB 68.5.0 (64-Bit) on a dovecot server. If you would like me to confirm the fix on the current TB version, could you please provide me with an installer with applied patch or let me know what else I could do to test the fix on a current version? Thanks!
|
||
Comment 17•6 years ago
|
||
Gene, can you please repeat the try builds.
|
|
Assignee | |
Comment 18•6 years ago
|
||
(In reply to Jorg K (GMT+1) from comment #17)
Gene, can you please repeat the try builds.
No problem. Working on something else right now. Will get on this ASAP.
|
|
Assignee | |
Comment 19•6 years ago
•
|
||
dreimal: You can find the "try" build here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5833eaeba94171e35e047c1b20c36a5c68e04970
Click on the green "B" next to the line for the arch and along the bottom you will see more stuff. Then select "Job Details" and select the file to download from the list that appears. (See more info in comment 13 but the links there are no longer valid.)
This try build applies my patch to the daily build so may not be 100% stable. I tried to apply my patch to the latest ESR 68 tree but I couldn't get it to build on the try-comm-central server after pushing to it.
Edit: If you run the daily build with your current 68 profile, when you go back to 68 you may need to define and run with the environment variable MOZ_ALLOW_DOWNGRADE and/or run tb with the command line option -allow-downgrade. Without this you will be prompted to create a new profile and will be unable to run 68. I think this comes from recent code inherited from mozilla. So you may want to run the daily build with a new profile to avoid this problem.
|
|
||
Comment 20•6 years ago
|
||
Thanks Gene!
I've installed the "try" build on a Win10 VM with a new profile.
This is the outcome with TB 68.6.0:
Folder structure in namespace "shared":
shared
shared / test
shared / test / shared folder
Moving folder "shared folder" into "shared" creates this folder structure:
shared
shared / test
shared / shared / shared folder
Now with the "try" build, moving folder "shared folder" back and forth beween the top-level "shared" folder and the "shared / test" subfolder works as expected. Thanks for fixing!
|
|
Assignee | |
Comment 21•6 years ago
|
||
Thanks for testing and glad it works. However, it's been a while since I made the patch so I need to reevaluate its correctness and submit a formal patch for approval. The "try"build is just for informal testing and not really a "fix" yet.
|
|
Assignee | |
Comment 22•5 years ago
|
||
I've done some more work on this but the hardest part has been duplicating the problem. I found that my current dovecot setup doesn't show the problem because the "personal" namespace, Inbox, has all its folders inside Inbox. This causes the problem to not occur. I had to change my dovecot server setup so that all the folders are at the same level as Inbox. After much trial and error, I found that I just had to comment out the "prefix" setting from the personal namespace setting in dovecot's 10-mail.conf. Also had to redo my tb profile to get tb to see the change. So now I see the problem again (and don't have to ask Bron for the test account again).
Doing more testing on this and don't see any bad effects of the change and it does fix the bug.
However, there is a note in the code here:
https://searchfox.org/comm-central/rev/84963a671bdff78d8d7ffc099e97072fe307d4d2/mailnews/imap/src/nsIMAPNamespace.cpp#212
This accounts for nested namespaces (i.e. "Public/" and "Public/Users/)"
Would someone really set up "nested" namespaces? Namespaces are bad enough without doing this too. :) I haven't found anything on even what a nested namespace is so if anyone knows, let me know. I don't see any tests for this either in tb test code.
|
|
Reporter | |
Comment 23•5 years ago
|
||
I don't think I've ever seen a server with a nested namespace (beyond the trivial cases of one of the namespaces being empty, like this):
- NAMESPACE (("" "/")) (("Other Users/" "/")) (("Other Folders/" "/"))
or:
- NAMESPACE (("INBOX." ".")) (("user." ".")) (("" "."))
I very much doubt there are many (if any) real world IMAP servers doing it.
Bron.
|
||
Comment 24•5 years ago
|
||
(In reply to gene smith from comment #7)
Tb does this:
. rename INBOX/a-b INBOX/Trash/a-b
but the server returns an error for this: "Renaming not supported across conflicting directory permissions." Maybe its a dovecot setup error on my part.I can only delete the new folders under Inbox if I set tb "delete model" to mark as deleted. Then it just does imap DELETE and not RENAME into Trash.
This sounds like my Bug 1628600.
|
||
Updated•3 years ago
|
|
|
||
Comment 25•6 months ago
|
||
(In reply to Tristan Miller from comment #24)
(In reply to gene smith from comment #7)
Tb does this:
. rename INBOX/a-b INBOX/Trash/a-b
but the server returns an error for this: "Renaming not supported across conflicting directory permissions." Maybe its a dovecot setup error on my part.I can only delete the new folders under Inbox if I set tb "delete model" to mark as deleted. Then it just does imap DELETE and not RENAME into Trash.
This sounds like my Bug 1628600.
FWIW that bug was closed INVALID a month after comment 24.
Description
•