Using jun8 commercial build Steps: 1. From mail window, File|New Folder. 2. Select Local Folders as the parent for the new folder. 3. Give the folder a name with a question mark in it, such as "What?". Confirm OK to create the folder, folder created and appears in Local Folder hierarchy. 4. Select the new folder. Result: Alert dialog appears "unknown error". After OKing the alert, I can copy messages to the folder, folder remains through exit.

moving to M18.

nominating for nsbeta3

- per mail triage

sorry for the extra email. Removing mail2 keyword.

It won't accept an "OK" for me, if the proposed foldername starts with a questionmark.

? is an illegal symbol in fat16, it's also generally a high risk symbol for file systems. I'd suggest wontfix. If we do intend to fix this, it means that mailboxes will have to store internal names that can differ from external names. and then what do you do when you have 2 folders w/ the same internal name? IE is cute. I'll attach a picture just for the sake of arguement.

cc Esther. She's seen with a comma "," in the folder name.

Additional information. When a user creates a Local folder with a comma in the name, we create a duplicate folder with a randomly generated name after closing and reopening app. 1. Launch mail 2. Create a new folder under Local Folders called: comma, this 3. Open the folder, you get the "unknown error 80520012", OK this error 4. Drag messge in that folder 5. Exit app 6. Launch mail again and view your Local Folder Result: You will have a duplicate of comma, this . The dup folder has a randome name something like this 8c798a9. It will have the message in it too. Expected: If the comma was illegal, we shouldn't allow the name. If the comma is legal, we shouldn't create a dup folder. Note: If this folder was migrated from 4.7 it will substitue the comma for an Underscore (example comma_this). This confuses the user, and I have found dataloss when you delete the random folder, it deleted the message in the original folder. If the user had done a shift delete or deleted the trash the message is not retrievable.

Adding Jenm.

reassigning to cavin, cc naving. We should handle all illegal characters.

IMAP new folder with "?" in name has been covered from bug 20324. This is for Local Folder -> Ccing sheela.

moving to 0.9.3 per pdt

updating summary. there is a related migration bug that I'm logging now.

migration issue spun into bug #87380

I believe we have to follow the same fix I made for news, see nsNewsFolder.cpp, CreateSubfolder(). you have to hash the name before you create the file for the db, and then set the name of the folder [folder->Name()] to be the "real name". We'll have to fix Rename(), CopyFolderLocal() CheckIfFolderExists()

The patch fixes the problem described in this bug (ie, creation of local folders with illegal chars). There are a few bugs related to local folder rename problems (64462, 87872, etc) which will be addressed by Navin. Just want to be clear that these problems are not caused by the patch. The way we implement the fix is that we 'hash' the illegal folder name and use it to create the physical files on disk. Then we store the illegal folder name in the folder cache file so that we can carry the name from one session to another. For example, assuming folder 'a,b' has the hashed folder name '3d350ca7', then files 'a4eb90c2' and '3d350ca7.msf' are created on disk and the name 'a,b' is stored in the folder cache file (panacea.dat). One problem I found and still don't a solution to is the following scenario: Folder 'a,b' contains a subfolder 'a?b' and you rename 'a,b' to 'ab'. After the rename is done you'll see that folder 'ab' now contains subfolder 'a4eb90c2' which is the hashed name for 'a?b'. The reason why this is happening is that we remove the cache info for all the subfolder of 'a,b' when we rename 'a,b' to 'ab'. So by the time we rebuild the subfolders for 'ab' we have no idea about the subfolders' original names and we can't hash 'a4eb90c2' back to 'a?b'. Same problem when you lose the folder name info in the cache file (panacea.dat). Another problem is that if you try to name 'a,b' to '3d350ca7' it'll fail because we hash 'a,b' to '3d350ca7' and then check if file '3d350ca7' exists. Not much we can do about this and since it's not very likely that this will happen so we should be fine with the issue.

> For example, assuming folder 'a,b' has the hashed folder name > '3d350ca7', then files 'a4eb90c2' and '3d350ca7.msf' are created on disk and > the name 'a,b' is stored in the folder cache file (panacea.dat). you mean: "then files '3d350ca7' and '3d350ca7.msf' are created", right? > One problem I found and still don't a solution to is the following scenario: > Folder 'a,b' contains a subfolder 'a?b' and you rename 'a,b' to 'ab'. start a new bug on this. I think it would be ok to check in with that problem outstanding. in the new bug we can talk about how to fix it. (somehow we're going to have to recursively remove the old folder cache elements and add new ones.) > Another problem is that if you try to name 'a,b' to '3d350ca7' it'll fail > because we hash 'a,b' to '3d350ca7' and then check if file '3d350ca7' exists. > Not much we can do about this and since it's not very likely that this will > happen so we should be fine with the issue. when it fails, does the user get an alert saying "you can't create that folders, since that folder already exists"? we can't just call "MakeUnique()" blindly, since that would allow users to create folders with the same (legal) name over and over. but, we might be able to get fancy and do "MakeUnique()" if it turns out the parent folder doesn't have immediate children with the same pretty name. but I agree it is not very likely, but start a new bug (and we'll punt on it), since it is not worth the effort right now. I'll go review the patch.

1. Yes, I meant "then files '3d350ca7' and '3d350ca7.msf' are created" (cut and paste error). 2. Yes, we do prompt users for the error (indicating that the folder already exists). I'll log these two problems.

comments: 1) I don't think onlineName is appropriate for local folders. local folders aren't online. (it makes sense for imap folders, which are actually online). I see that dbfolderInfo uses "onlineName", so we can't change that. but perhaps in localmailfolder.cpp, we can come up with a different name. 2) + if (NS_SUCCEEDED(rv) && (const char *) onlineName && nsCRT::strlen((const char *) onlineName)) do use .get() instead of casting. 3) to be safe, in WriteToFolderCacheElem() and ReadFromFolderCacheElem() you might want NS_ENSURE_ARG_POINTER(element); unless the caller checks for null before calling. 4) + nsXPIDLCString onlineFullUtf7Name; + rv1 = cacheElement->GetStringProperty("onlineName", getter_Copies (onlineFullUtf7Name)); + if (NS_SUCCEEDED(rv1) && onlineFullUtf7Name.get() && nsCRT::strlen (onlineFullUtf7Name.get())) + currentFolderNameStr.AssignWithConversion(onlineFullUtf7Name.get()); this code (looks copied from the imap implementation) is misleading. onlineName is not really a UTF7 string. some of the string converting that is going on (from CString to String and back) has me worried for the non ASCII case. you've made the "onlineName" attribute a string property, like it is for imap. that works for imap, since the online name is in modified UTF7, which is 7 bit clean and we convert it later before coming up with the PRUnichar * version of the name. (note, for dbfolderInfo, it is a PRUnichar * property) in nsLocalMailFolder.cpp, when the user creates a folder, folderName is a PRUnichar *, and we convert that to a CString. if I tried to create a non ASCII local folder name, would would happen? we might have to take the PRUnichar *, and turn it into escaped UTF8 (which is 7bit clean), store that as the "onlineName" property, and then pulling it out of the folder cache, turn it back into a PRUnichar *.

This fix would be nice to have. My biggest concern is that it affects how we create name for local folders (which is most of the users out there) and the patch is relatively big. This is one of the ones I think we need to feel very confident about before taking onto the branch.

the "rename, start" bug that cavin is going to log is very similar to a bug naving owns for imap folders. (#70517)

This patch only supports 7bit ASCII and the 1st problem described above is also fixed: Folder 'a,b' contains a subfolder 'a?b' and you rename 'a,b' to 'ab'. In addtion, the problem of not showing the correct folder display names when panacea.dat is missing or corrupted is fixed. One new problem that was discovered and has not been fixed is folder names with slash(es) (like 'a/b'). For now, code is added to disallow folder names with slash(es) to be created or renamed. An error dialog is displayed in this case. I'll file a new bug against it. I'll also have to try the patch on linux and mac to see if I need to change the special char list in CheckIfContainsSpecialChars() (ie, to see if there are any chars that don't work on these two platforms). Loading the latest code now.

minor comments. over all, it is looking good. 1) cavin and I decided to move the existing bug (the "a/b" problem) out to another bug. see #89986 so the plan is to remove that part of the fix from this patch. 2) + rv = GetDBFolderInfoAndDB(getter_AddRefs(folderInfo), getter_AddRefs(db)); + // do this after GetDBFolderInfoAndDB, because it crunches m_displayFolderName (not sure why) any idea yet why GetDBFolderInfoAndDB() stomps on m_displayFolderName()? 3) + folderInfo = null_nsCOMPtr(); folderInfo is a COMPtr, so once the method returns and folderInfo goes out of scope, it will release its ref. I don't think you have to do it manually. 4) +NS_IMETHODIMP nsMsgLocalMailFolder::GetDisplayName(PRUnichar ** aDisplayName) +{ + if (!aDisplayName) + return NS_ERROR_NULL_POINTER; just do NS_ENSURE_ARG_POINTER(aDisplayName); 5) + ReadDBFolderInfo(PR_FALSE); // update cache first. ReadDBFolderInfo() returns a nsresult. we don't care about the return result? if we really don't care what the return value is, we should do this: (void)ReadDBFolderInfo(PR_FALSE); 6) before landing, we should double check that we haven't broken any existing functionality for users on with non latin-1 or non ASCII system charsets. it looks like we should be ok, but it would be bad if this fix makes it so those users who could create non-ASCII folders are now unable too. nhotta, can you help test or help review when we've got a final patch?

I can review the next patch. I am wondering why aren't we fixing to support the unicode folder names or is it taken care of in another bug.

FYI: |null_nsCOMPtr| is deprecated. If you plan to use it, just use 0 instead.

> I am wondering why aren't we fixing to support > the unicode folder names or is it taken care of in another bug. we'll spin that issue of to another bug. currently, we only support local folders that have ASCII names or names in the system char set. cavin and I see a way to extend his fix to allow for local folder names to be in ANY languange no matter what the system charset, but that will take more work and more testing, so we'll wait on it until after this fix is in. (assuming folders in the system charset still works.)

to elaborate on + ReadDBFolderInfo(PR_FALSE); unless there is a good reason not to check rv (for example, we expect to fail in certain cases, so we don't want to return error if that call fails) this should be: rv = ReadDBFolderInfo(PR_FALSE); // update cache first. NS_ENSURE_SUCCESS(rv,rv); otherwise, do this: // ignore failure, because... (void)ReadDBFolderInfo(PR_FALSE); // update cache first.

Looks like we don't need the ReadDBFolderInfo() call in GetDisplayName(), so I'll remove it before I check in the code.

why don't we use nsresult nsMsgDBFolder::CreateFileSpecForDB for hashing for msf file rather than adding new code here.

Navin, you're right and I already changed the code to make use of CreateFileSpecForDB(). So can I get a review from Navin and Seth so that I can check in the code? Let me know if you want to see the updated patch again.

please attach the updated patch. I would like to take a look

Is this safe? - if (NS_FAILED(ConvertFromUnicode(fileCharset, nsAutoString(aNewName), &convertedNewName))) + nsAutoString tmpAutoString; + tmpAutoString.AssignWithConversion(newSafeName); + if (NS_FAILED(ConvertFromUnicode(fileCharset, tmpAutoString, &convertedNewName))) before your fix, we used aNewName, which was the PRUnichar * that they user typed in. (could have been the folder name, in chinese.) now, we are converting newSafeName, which is going to be a hashed version that might be altered because of illegal chars or illegal length. what do we do with convertedNewName after this?

convertedNewName is the new folder name to be renamed later in the code. Actually, newNameStr and convertedNewName are used interchangeably but I have changed the code so that only newNameStr is used. We need to hash the folder name in order to support folder name like 'a,b'. Not sure if we need to call ConvertFromUnicode() if we only support ASCII chars for now. To support non ASCII chars like Chinese we need to do something different which will be addressed in a different bug. I'll attach a different patch per naving comments.

>Not sure if we need to call ConvertFromUnicode() if we only support >ASCII chars for now. Is this about local folder name? Non ASCII is supported for local folder name.

sorry for the delay. I'm testing the latest patch on roy's japanese win2k machine. -sspitzer

actually, roy showed me how to change the system char set on my win2k box. I'll be testing the patch out there, and I'll see that migration of 4.x non- ASCII local folder names still works.

cavin and I found some problems on non-ASCII operating systems. we're fixing it now, new patch coming soon.

here's a todo list for that patch. 1) not sure why rename of non ascii local folders works (see 1302 in nsLocalMailFolder.cpp) 2) name of file on disk if not in same char set (see "XXX TODO" in hashifnecessary). (not necessary for this fix, but getting that work should allow users to name local folders to non-ASCII and non-system charset values. japanese folder names on a US-ASCII or Big5 OS) 3) Get/Set MailboxName not wstring friendly, so we use escaped UTF8 4) no Get/Set PRUnicharStringProperty on folder cache element, so again use escaped UTF8 5) code cleanup

heads up, cavin has found a problem with my last patch. StringHash() needs to be updated to handle null bytes (since it is now hashing a PRUnichar *, and not a char *. he's working on that.

cavin, can you elaborate (for nhotta benefit and others) on why we had to comment out that code? I'm concerned that certain strings in japanese will still cause the same problem we saw when we turned that code on. I'm most interested in the second code path, the one that goes through NS_MsgHashIfNecessary() instead of the new NS_MsgHashAutoStringIfNecessary() I started looking into making the changes we'd need to deal with that (switching more code to nsAutoString and nsILocalFile) and it quickly got out of hand.

last night, I tried make the changes necessary to fix the problem we ran into where we'd use NS_MsgHashIfNecessary() in one code path, and NS_MsgHashAutoStringIfNecessary() in another. most of it involved replacing nsFileSpec / nsIFileSpec with nsILocalFile, so I could use nsILocalFile's appendRelativeUnicodePath() method. I stopped when I got here: 399 rv = nsLocalURI2Path(kMailboxRootURI, folderURI, folderPath); 400 401 if (NS_SUCCEEDED(rv)) 402 { 403 // set up the url spec and initialize the url with it. 404 nsFilePath filePath(folderPath); // convert to file url representation... 405 406 if (mPrintingOperation) 407 urlSpec = PR_smprintf("mailbox://%s?number=%d&header=print", (const char *) filePath, msgKey); I'm not sure what to do there yet. filePath is a unix style path to the folder. If we've got a nsILocalFile, I think we'd have to use nsIFile's path or URL attributes to get what we want, but I'd need to check that it does the right thing on all platforms. I'm also nervous about what it will return if the file is non-ASCII. I'd also need to see how we parse that url back out. assuming cavin's last patch doesn't break any existing functionality on japanese systems, we should work towards landing it, and later resolving the NS_MsgHashIfNecessary() / NS_MsgHashAutoStringIfNecessary() issue that we found.

see bug #68993 "filename containing Shift-JIS trail byte 7C" I'm worried about that happening with the current patch. if the folder name has that, the call to NS_MsgHashIfNecessary() will hash it, but the call to NS_MsgHashAutoStringIfNecessary() will not, so we'll fail to open the folder when we click on it. (we'll get that same error that cavin saw when he uncomments that code.)

I am not sure why there are two versions. But is that possible to convert to the OS charset instead of AssignWithConversion? If the conversion fails then the name has to be hashed. nsresult NS_MsgHashIfNecessary(nsCAutoString &name) { + nsAutoString autoStr; + autoStr.AssignWithConversion(name); + nsresult rv = NS_MsgHashAutoStringIfNecessary(autoStr);

OK, here are the reasons why: 1. In nsMsgLocalMailFolder::CreateSubfolder() we call NS_MsgHashAutoStringIfNecessary() directly to make sure that we have a safe folder name to create .msf file and the msg file (ie, 'aa.msg' and 'aa'). The string passed to NS_MsgHashAutoStringIfNecessary() is the one entered by the users (ie, no conversion is performed on it) and for this reason nsMsgI18Ncheck_data_in_charset_range() returns TRUE because Chinese string is supported by Chinese windows. At this point we have created .msf and msg files on disk. So far so good. 2. Then in nsMsgLocalMailFolder::CreateSubfolder() we call AddSubfolder() which eventually calls NS_MsgCreatePathStringFromFolderURI(). The latter does some string manipulations and calls NS_MsgHashIfNecessary(). NS_MsgHashIfNecessary() then converts the input C string to nsAUtoString before calling NS_MsgHashAutoStringIfNecessary(). So by the time we try to check if the folder name will convert to the underlying OS the string is altered and the call to nsMsgI18Ncheck_data_in_charset_range()fails! Since the call fails the name is hashed and all the subsequent calls to open the folder database fails as well. This is because in step one we create the .msf file with the original name the users entered and now we try to open the database with a different (hashed) name. The problem is really that the string passed to NS_MsgHashAutoStringIfNecessary() (called by NS_MsgHashIfNecessary()) does not represent a correct Chinese string. To fix the problem we need to make NS_MsgCreatePathStringFromFolderURI() call NS_MsgHashAutoStringIfNecessary() directly (instead of NS_MsgHashIfNecessary()). But then we need to change NS_MsgCreatePathStringFromFolderURI() to use nsAutoString instead of nsCAutoSting. Commenting out the call to nsMsgI18Ncheck_data_in_charset_range(), the patch works for the following (tested) cases of folder creation: 1. ASCII folders in English windows. 2. Chiness folders in Chiness windows. However, with the call to nsMsgI18Ncheck_data_in_charset_range() it does not work for the following cases of folder creation: 1. Chiness folders in English windows. 2. Chiness folders in Chiness windows.

>NS_MsgHashIfNecessary() then converts the input C string to nsAUtoString before >calling NS_MsgHashAutoStringIfNecessary(). This is done by AssignWithConversion as I mentioned in my last comment This may lose the data because it just casts instead of applying charset conversion. I think utility like ConvertToUnicode() can be used there to convert to unicode from the OS charset. If the conversion fails then the name needs hashing.

moving to 0.9.4

adding nsenterprise-

Found anther issue when running on the Japanese windows. If the folder name contains a Japanese char whose double byte code is '837C' (note that '7C' is '|' which is an illegal char on win32 in the code) then we'll end up hashing the folder name and we'll get 'Unknown error' msg when opening the folder. So in this case we should not hash the folder name. But if I enter 'a|b' then we'll want to hash the name. Question is how does the code tell if a particular '7C' char comes from a Japanese char or it's from the ASCII '|'?

we'd like this for Mach V but it really isn't something we need for eMojo. Mail news triage meeting --> .9.5

moving to 0.9.6

I need an unicode version of NS_MsgHasIfNecessary for when we save an attachment. It will be used in nsMessenger::SaveAttachment and in nsMessenger::SaveAllAttachments. Let me know when you are done with this bug.

moving to 0.9.8

*** Bug 115777 has been marked as a duplicate of this bug. ***

*** Bug 125868 has been marked as a duplicate of this bug. ***

*** Bug 188390 has been marked as a duplicate of this bug. ***

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and <http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss bugs are of critical or possibly higher severity. Only changing open bugs to minimize unnecessary spam. Keywords to trigger this would be crash, topcrash, topcrash+, zt4newcrash, dataloss.

Folder of "?" could be created and used with no error by both Mozilla Suite latest-trunk(1.8a5) and Thunderbird 0.9(and latest nightly) on Win-2K. ( file set of "HEXA-string" and "HEXA-string.msf" was created ) Mscott has successfuly resolved this lo----nglived bug, I think, by fix for Bug 219586 and Bug 264467, post fix of Bug 264071 is required though. Is it right? (Note : "/" and "?" problem disappered, but "#" problem still exists)

*** Bug 239502 has been marked as a duplicate of this bug. ***

WFM on thunderbird 2.0 beta 1/windows and Thunderbird/2.0b1 ID:20070108 on linux. (? in imap folder names seems fine too, but someone here said that's bug 20324)

Based on Comment 69 => WFM