From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.2) Gecko/20010725 BuildID: 2001072504 The messages in a folder named test%25test are not displayed in detail. Reproducible: Always Steps to Reproduce: 1. Create a new subfolder named test%25test (Yes, I know it is evil, but ...) 2. Move a message into this folder 3. Try to display the message in the new subfolder Actual Results: The message is not displayed. Expected Results: Display the message. This also happens on the trunk, where I see the following messages: ###!!! ASSERTION: couldn't get message header: 'PR_FALSE', file nsMailboxProtocol.cpp, line 389 ###!!! Break: at file nsMailboxProtocol.cpp, line 389 Error reading file [...ommited...]/test%test Seems to me the url is unescaped when it should not.

I stand corrected. It is not unescaped to much, we do not escape to much. In nsMailboxService.cpp there is in at least three times code that looks like this: rv = nsParseLocalMessageURI(aSrcMsgMailboxURI, folderURI, &msgKey); rv = nsLocalURI2Path(kMailboxRootURI, folderURI, folderPath); if (NS_SUCCEEDED(rv)) { // set up the url spec and initialize the url with it. nsFilePath filePath(folderPath); // convert to file url representation... urlSpec = PR_smprintf("mailbox://%s?number=%d", (const char *) filePath,msgKey); nsCOMPtr <nsIMsgMailNewsUrl> url = do_QueryInterface(*aMailboxUrl); url->SetSpec(urlSpec); PR_FREEIF(urlSpec); } An URI is converted into a local Filepath and then back into an URI. Up to the folderuri the uri is properly escaped, after the conversion to a FileSpec it is unescaped (as it should), then the unescaped filePath is directly used to build the new urispec. That is wrong, it has again to be escaped before called with SetSpec. Inserting this code (quick hack, copied from nsPop3Service) after the construction of filepath nsXPIDLCString escapedFilePath; *((char**)getter_Copies(escapedFilePath)) = nsEscape(filePath, url_Path); and then using escapedFilePath instead of filePath, fixes the problem. I can only guess that there are much more places inside mailnews where this url -> filepath -> url conversion is done. Alls this need to be looked at.

not a database problem. It's a folder unescaping problem. You're talking about local mail, I assume. Seth, who was working on these issues? Was it Cavin?

reassigning to Cavin. he's got other bugs related to issues like this.

Local folder 'a#b' has the exactly same problem. In this case, the error msg indicates that the folder it tried to open was 'a' (instead of 'a#b'): Error reading file [...omitted...]/a The reason why the resulting folder is 'a' is that the parsing code in nsStdURLParser::ParseAtDirectory() uses ';', '?' and '#' as delimiters to figure out file and directory names. So looks like we need to escape the folder name before calling: url->SetSpec(urlSpec);

I just noticed that nsLocalURI2Path() was added the code to return unescaped folder name in the output parameter 'pathResult'. The changed was made on 09/26/2000 by nhotta to fix 52165 (in file local/src/nsLocalUtils.cpp). So it looks we may have a dead-lock situation here because we need the unescaped folder names in one case but in other cases we don't need it. We'll have to try both 7 bit and 8 bit folder names when we have a fix for this. Need to investigate it more.

I feared it would not be that easy. Also I found it very interesting what happend when my first attempt at the problem (described above) got accidentally checked in with my fix for 40670. I tested on linux and it worked fine, but it didn't apparently on windows: With an escaping issue????? Really strange! When creating a new folder the name is typed normally. The folder is created as typed. In the middle is a mailbox url where the normal url rules apply, so escaping is necessary with SetSpec. That is absolutely nesessary. No way around that. The message database seems to use the real name as key. So unescaping the (previously escaped) url is necessary to access the message. Is the problem in bug 52165 a porting issue from 4.x to mozilla? Were the folders differently encoded on 4.x? If it is, maybe the deadlock can be solved by renaming the folder when moving from 4.x to mozilla.

>I feared it would not be that easy. Also I found it very interesting what >happend when my first attempt at the problem (described above) got accidentally >checked in with my fix for 40670. I tested on linux and it worked fine, but it >didn't apparently on windows: With an escaping issue????? Really strange! > My guess is that it's because we also escape "C|" at the beginning of 'filePath'. "C:" is turned into "C|" in the url. Since nsLocalURI2Path() is called only by PrepareMessageUrl() we can comment out the unescaping folder name code in nsLocalURI2Path() to achieve the same goal. I have not debugged the problem yet so I don't know if this is going to work.

As far as I know the C: <-> C| stuff is not part of escaping. Maybe the real root of the problem is bug 33451.

Update: this still happens with current build on linux and mac local mail subfolder with test%25test name. I'm not sure if it's mentioned in this bug or not, but I can see the msgs in the thread pane, but nothing (no header or msg body) shows up in the Message pane.

I started to look into this again and I think all we need to do is replace the call for nsEscape with a call to NS_EscapeURL (with esc_Forced) in NS_MsgEscapeEncodeURLPath located in mozilla/mailnews/base/util/nsMsgUtil.cpp to get the right escaping on already escaped looking strings.

No it is not enough, the other problem from comment #1 still needs to be adressed.

This patch also fixes problems with foldernames like a%20b or a#b. The escaping used is very minimal so hopefully nothing breaks. I tested it on linux an could not find any regressions. Can anybody with a windows build test it.

Comment on attachment 127959 [details] [diff] [review] recreated patch to prevent bitrot trying another reviewer

I can try this on windows.

Andreas Otte, will your patch resolve following "#" related 2 bugs too? Bug 94124 Local Folder: Can create folder "#ab" but can't see it on restart.. Bug 115091 Mail subfolder can not have a # sign in the name..

Yes, I think so ...

David, any test results?

I applied the patch and rebuilt, and I still have the problem with a#b. Test%25Test worked, however.

You are correct, it does not work anymore with a#b. I'm sure it did at one point in time. Thanks for testing, will try to fix it.

bienvenu: Apparently you checked in this changes by accident while working on bug 137006. Status of this patch: Works for %25 or similar, does not work for a#b. I had no time to look after this ...

ah, thx for pointing that out - I couldn't remember what bug # that patch was for. If I get a chance, I'll try to figure out the a#b problem.

some or all of this patch will need to get backed out. The problem is that the uri's for msg folders are stored in the msg filters.dat file, and any change in the escaping of uri's is going to break some msg filters. This caused the regression bug 229345 - my bad. I'll need to figure out if we can upgrade the filter uri's, perhaps by unescaping them and then re-escaping them correctly.

Hmmm ... changes in escaping/unescaping or fixing the order of escaping/unescaping or fixing missing escapes or unescapes can happen every time ... so those filter uris really need to be fixed, backing this out can only be temporary.

I agree that it should be only temporary, but we store uri's in pref.js as well as msgFilterRules.dat - if the URI's can change at any time, they're no good as a persistent representation of folders. So far, this is the first time anyone has broken us, so I'm a little more optimistic that they won't be changing frequently.

Maybe the only problem is that we are missing there an escape or unescape as it was the case with this problem. Adding it here might force adding it there.

How about changing folder name and real file name on folder creation. Netscape 4.x avoided special character in folder name problem by changing file name. For example, if folder name contains "\" in MS Win environment, Netscape 4.x changes it to string like "_A", "_B" if "_A" exists, in file name. Mozilla has already mechanism for different folder name and file name, keeping folder name in ".msf", and this mechanism is already used on illegal character in folder name case. Since internal mail folder path uses real file name, this bug's case, character string same as escaped character sequence, can be avoided if file name is changed to different string, for example "%_25". I believe this Netscape 4.x technique is applicable on all remaining problems of illegal or special character in folder name, problems for "/?#" and "~" and ".", if someone have submarine patent right.

Correction. Sorry for spam. (Invalid) > if someone have submarine patent right. (Valid) > if nobody has submarine patent right.

In addition, if above Netscape 4.x technique is used, "hex string file name" problems can be avoided. When folder name contains "\", Mozilla currentry creates file name of hex string based on Unicode under MS Windows-Me. For example, for folder name of "A\B", Mozilla 20031228-trunk/Win-Me created file/directry named "6e127717", "6e127717.msf" and "6e127717.sbd". This ununderstandable filename sometimes causes confusion of users. (1) When user deleted ".msf" file instead of using "Compact this folder" in order to re-create ".msf" file, folder name becomes the hex string. (2) When user imports mails from othe Mailer and mail folder name contains illegal characters, file name for the folder becomes/contains hex string. User impact on these situations can be reduced by Netscape 4.x technique. Even if "A\B" is changed to "A_|_B", many users can guess this file is for folder "A\B", or was for folder "A\B".

Sorry but Netscape 4.x technique in Comment #29 does not applicable to IMAP folder created by oter than Mozilla.

I did back out this patch - making the filter code handle changes in uri escaping wasn't trivial, and we need to fix that before we can check this in.

How about to comletely separate (A) "Folder Name" and (B) "Folder File Name(Physical Path Name)" and (C) "Internal Mail Folder Path Name(URI format)? (A) and (B) (almost equal to (C) currently) are separated already - (A) is saved ".msf" file if folder name is different from file name (illegal file name character case). Separate folder file name("%25" in this bug) and folder internal path name(URI format). For example, assign unique mail folder number and use this number in internal mail path(URI format) used in prefs.js or msgFilterRules.dat. Needless to say, relationship between folder number and pysical file name should be kept in somewhere and conversion from one to another should be done easily. This unique identifier can be "Short Folder Name" or "Folder Nick Name". If unique short name(nick name) of the folder does not contain any unsafe special characters, it can be used in both file names and URL with no problem. And short folder name forces user to convert special characters in folder name to safe file name characters and safe URI characters by himself instead of automatic conversion by Mozilla. It is clever way, I think. In addition, this can easily resolve not only all special URI character related problems("/","#","?" in addition to "%") but also starting "." and ending "." in file name problems. However, if folder is IMAP folder and is defined by other mailer, folder file name may contain unsafe characters. So (A) "Folder Name" and (B) "Folder File Name(Physical Path Name)" and (C) "Internal Mail Folder Path Name(URL format) should be distinguished. Since current mechanism, real folder name in ".msf", may causes difficulty in mail folder recovery, relationship among "Short name" and "File Name" and "Long Name" should be kept in somewhere else. I think text file in Mail directry is better place than prefs.js. Changes required to support : (1) New folder creation (POP and IMAP) Force user to enter "Short Folder Name". Short folder name : used in URI Physical file name : same as Short Name Long folder name : Already user entered (2) Import Add promt logic for "Short Folder Name" if file name contains unsafe characters. Short folder name : used in URI (New) Physical file name : same as Short Name Long folder name : can be changed from old physical file name (3) Subscribing already defined IMAP folder Add promt logic for "Short Folder Name" Short folder name : used in URI Physical file name : as is Long folder name : can be changed from physical file name Note: Starting "." and ending "." in file name problems may not be resolved if folder is already defined at server. (4) When unsafe file name is found in mail directry on restart. (User can manualy copy or rename folder file) Ignoring and warning message is sufficient, I think. But for migration, renaming support while restart is preferable.

Added this bug to dependency tree of bug 124287 for ease of access.

this seems to work for me today with a trunk build on windows.