1608318 - nsImapUrl::EscapeSlashes() <=> nsImapUrl::UnescapeSlashes() roundtrip can lose chars.

Status: NEW

(MailNews Core :: Networking: IMAP, defect)

Description

[Ben Campbell]

nsImapUrl::EscapeSlashes() performs these replacements:

/ => ^
^ => ^^

nsImapUrl::UnescapeSlashes() performs these replacements:

^^ => ^
^ => /

So if you have a starting string containing multiple slashes, an EscapeSlashes, followed by UnescapeSlashes() won't end up with the original string... eg:

nsImapUrl::EscapeSlashes("foo//bar") => "foo^^bar"
nsImapUrl::UnescapeSlashes("foo^^bar") => "foo^bar"

Comment 1

[Ping Chen [:rnons] (away)]

I see this problem when working on imap-js, will use encodeURIComponent/decodeURIComponent for now.

Comment 2

[Ben Campbell]

I think the main thing is to be really really aware of the context of each name, and be explicit when converting names between contexts.
(IMAP mailbox names have very different rules from TB folder URIs, for example).

I'd even be inclined to make up specific conversion routines (eg IMAPMailboxToFolderURI() to convert from a IMAP folder to a TB folder URI), even if they are just shims for encodeURIComponent/decodeURIComponent. This keeps all the conversion rules in one place and makes it really clear when such a conversion is taking place.

At the moment such conversions are done ad-hoc, all over the place.
The multitude of bugs hanging off Bug 124287 are a reminder of what happens when we get a bit loose about such things :-(
There are some possibly-relevant notes in https://bugzilla.mozilla.org/show_bug.cgi?id=124287#c26

If we just assume users will have the most annoying possible folder names we'll be fine - lots of non-ascii folder names with spaces, filesystem-illegal chars, and with "/" as part of the name rather than a separator :-)