229345 - filters with action to move to folder with "@" in the name disabled

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[(not reading, please use [e-mail] instead)]

filters with action to move to folder with "@" in the name disabled

I just built and ran the trunk (12-23-03), but this might have regressed a while
ago.

I have a pop filter that moves mail addressed to [e-mail] into a local
folder named (visually) "[e-mail]".

here's how it looked in msgFilterRules.dat

name="[e-mail]"
enabled="yes"
type="1"
action="Move to folder"
actionValue="mailbox://[e-mail]/mozilla/staff%40mozilla.org"
condition="AND (to or cc,contains,[e-mail])"

note, in the uri, the "@" is escaped.

when I started up my 12/23 build and got new mail, it gave me that warning about
the action no pointing to a valid folder, and it disabled the filter.

I went in and manually reset it, and here's my filter now:

name="[e-mail]"
enabled="yes"
type="1"
action="Move to folder"
actionValue="mailbox://[e-mail]/mozilla/[e-mail]"
condition="AND (to or cc,contains,[e-mail])"

notice, the "@" is not escaped.

Comment 1

[WADA:World Anti-bad-Duping Agency]

Attachment: msgFilterRules.dat - both rules defined on 2003-12-17 and 2003-12-28 build

Comfirmed with 2003122809-trunk(zip for Win)/Win-Me. (2003-12-24 build also) 

(Case-1 : "!", problem occurs)
  (1-A) by old code (2003-12-17 build or before):
	actionValue="mailbox://[e-mail]/%21-TEST"
  (1-B) by new code (2003-12-24 or 2003-12-28 build):
	actionValue="mailbox://[e-mail]/!-TEST"

(Case-2 : "@", problem occurs)
  (2-A) by old code (2003-12-17 build or before):
	actionValue="mailbox://[e-mail]/%40-TEST"
  (2-B) by new code (2003-12-24 or 2003-12-28 build):
	actionValue="mailbox://[e-mail]/@-TEST"

(Case-3 : "%", problem does not occur)
  (3-A) by old code (2003-12-17 build or before):
	actionValue="mailbox://[e-mail]/%25-TEST"
  (3-B) by new code (2003-12-24 or 2003-12-28 build):
	actionValue="mailbox://[e-mail]/%25-TEST"

Because of above inconsistency in msgFilterRules.dat,
(Proble-1)
   When migrate to new code, user have to re-define message filters
   (re-specify target folder in message filter definition)
   if folder name contains special character such as "@" or "!".
(Problem-2)
   If user re-defined message filters due to (Problem-1)
   or user defined new filters on new code,
   user have to define message filter agani
   when falls back to old code.

(Problem-1) can be resolved if new code will parse esacped "@" or "!" in folder
path in filter rule.
However, (Problem-2) will not be resolved until new code will create compatible
message filter rule.

Comment 2

[David :Bienvenu]

1.6a code wrote out the uri escaped in msgFilterRules.dat, and unescaped it
before looking it up, I guess. The trunk code writes out the uri unescaped, and
thus the lookup fails. I don't know what change is responsible for this...

Comment 3

[David :Bienvenu]

ah, it's this checkin:

http://webtools.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=FilterEditor.js&branch=&root=/cvsroot&subdir=mozilla/mailnews/base/search/resources/content&command=DIFF_FRAMESET&rev1=1.77&rev2=1.78

Comment 4

[David :Bienvenu]

Jingshik, it looks like calling encodeURIComponent is not sufficient. Do we need
to keep the call to escape, and then call encodeURIComponent? We need the URI to
be escaped...

Comment 5

[Max Alekseyev]

Bug 229608 seems to be dup of this one. But it's about ``!'' symbol in the
folder name.
Likely the bug is not about ``@'' specifically but about any symbol that is
escaped in the URI.
If so, please dup and update summary accordingly.

Comment 6

[Max Alekseyev]

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

Comment 7

[Jungshik Shin]

David, thanks for the alert.

ECMA 262 has the following
(http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf)

uriReserved ::: one of ; / ? : @ & = + $ ,
uriUnescaped ::: uriAlpha DecimalDigit uriMark
uriEscaped ::: % HexDigit HexDigit
uriAlpha ::: one of
a b c d e f g h i j k l m n o p q r s t u v w x y z
A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
uriMark ::: one of - _ . ! ~ * ' ( )

It's strange that '@' is not escaped by encodeURIComponent(). As for '!', it's
not supposed to be escaped by encodeURIComponent().  

Anyway, to keep the backward compatibility, we have to escape the exactly the
same set of characters as before. I'll track them down  (in bug 44272)

Comment 8

[Jungshik Shin]

WADA, can you make a filter rule involving a non-ASCII folder name (Japanese)
and tell me what msgFilterRules.dat is like in 2003-12-17 build or before? Thanks.

Comment 9

[Jungshik Shin]

According to comment #1, it worked fine before 2003-12-17. My patch for bug
225695 got landed on 2003-12-03 (bug 225695 comment #63). So, that cannot be the
cause. I found that nsMsgFilterList.cpp had been updated on Dec. 19th
(http://lxr.mozilla.org/mozilla/source/mailnews/base/search/src/nsMsgFilterList.cpp),
which is more likely to be the cause.

BTW, I inserted 'dump' statement in the file refered to in comment #3 and '@' is
escaped properly as '%40'. (a simple html file with Javascript confirmed that
encodeURIComponent works per spec.) '!' is not (because encodeURIComponent is
not supposed to escape it). However, that shouldn't make any difference to
end-users because escaping/unescaping should be done to and from actual
filenames in any way. 

Comment 10

[David :Bienvenu]

the change on dec 19th was whitespace only: here are the non-whitespace diffs:

http://bonsai.mozilla.org/cvsview2.cgi?command=DIFF&subdir=mozilla%2Fmailnews%2Fbase%2Fsearch%2Fsrc&file=nsMsgFilterList.cpp&rev1=1.82&rev2=1.83&whitespace_mode=ignore&diff_mode=context

I'll try backing out the whitespace change, and I'll try backing out your
change, and see if either makes a difference...

Comment 11

[Jungshik Shin]

I think neither is responsible for this regress. (I also confirmed it works fine
with 2003-12-08). The change made to nsMailboxservice.cpp on 19th is more likely
to be 'the' cause.
(http://lxr.mozilla.org/mozilla/source/mailnews/local/src/nsMailboxService.cpp)

Comment 12

[Jungshik Shin]

Sorry for spamming. That was a part of the patch for bug 137006. This change
made in 
nsMsgBaseUtils.cpp is suspicious.

-  *result = nsEscape(NS_ConvertUCS2toUTF8(str).get(), url_Path);
+  nsCAutoString escapedStr;
+  NS_EscapeURL(NS_ConvertUCS2toUTF8(str), 
+               esc_FileBaseName|esc_Forced|esc_AlwaysCopy,
+               escapedStr);
+  *result = nsCRT::strdup(escapedStr.get());

Comment 13

[David :Bienvenu]

they're all suspicious, but none of them seem to get called when filters are
written out...

Comment 14

[WADA:World Anti-bad-Duping Agency]

Attachment: msgFilterRules.dat No.2(Shift_JIS folder/file name) FileName - both rules defined on 2003-12-17 and 2003-12-28 build

To Comment #8 From Jungshik Shin :

This is Shift_JIS foldername/filename case.
Please note that my environment is Win-Me and Shift_JIS is used as file name.
Since 1st byte of Shft_JIS code is greater than 0x81 and 2nd byte of Shift_JIS
is greater thsn 0x41, !(0x21) and @(0x40) does not appear in Shift_JIS code.

Comment 15

[Jungshik Shin]

Thanks. And sorry that it turned out that it's not relevant. BTW, it's always
UTF-8 that's used whatever local file system encoding may be. ('%E3%81%82'
represents one Japanese character) 

Comment 16

[David :Bienvenu]

on my trunk builds, I'm not seeing the folder uri get escaped, but the filters
remember their location. My tree might not be fully up to date, though, so I'm
doing a complete update.

Comment 17

[Jungshik Shin]

David, that's what I'm also seeing on my trunk build and what was reported. '@'
and '!' are not escaped, but folders with them (unescaped/bare) just work fine
with a trunk build. The problem is that old filter rules (saved by a build
before 2003-12-17 ~ 19) with escaped folder names (%40, %21) don't work with a
post-2003-12-20 (?) build. So, it seems like it's unescaping  somewhere....
  

Comment 18

[David :Bienvenu]

so new filters work fine, but old filters get disabled. So the code that looks
up folder uri's doesn't handle the case where '@' is escaped. It's not a general
escaping issue - space is still escaped and handled correctly...

Comment 19

[David :Bienvenu]

I don't think it's an unescaping issue - I think it's a different set of
expectations about what characters are escaped. RDF now seems to think that '@'
and '!' should not be escaped so it won't match uri's that have them escaped.
Or, perhaps the problem is that the mailnews code that creates the folder
resources no longer escapse '@' and '!' when creating the resource. In fact, the
latter would explain it all.

Comment 20

[David :Bienvenu]

which means it must be the change to NS_MsgEscapeEncodeURLPath, as you said.

Comment 21

[David :Bienvenu]

Attachment: proposed fix

undo part of checkin that caused this regression. I think this will undo a fix
for handling of certain folder names, but we need to fix that a different way.

Comment 22

[Brendan Eich [:brendan]]

Nominating for 1.6.

/be

Comment 23

[David :Bienvenu]

this change didn't go into 1.6, to the best of my knowledge, so 1.6 should be
fine as is.

Comment 24

[Brendan Eich [:brendan]]

Sorry, I thought jshin's big, brave escape/unescape purge did go into 1.6.  Hmm.

/be

Comment 25

[David :Bienvenu]

the regression was introduced with the fix to bug 137006, which is 1.7 only.
It's also not required for bug 137006. I'm trying to find the bug that it was
supposed to fix...

Comment 26

[Jungshik Shin]

My patch went into 1.6, but it's not the cause of this bug and 1.6 is fine as
David wrote.

Comment 27

[chris hofmann]

so we need to change this to blocking1.6 -  right?

Comment 28

[lohphat]

Pretty pleeeease?  ;-)   It's getting on my nerves.

Comment 29

[David :Bienvenu]

blocking 1.6- (can I do that? :-)  here goes)

Comment 30

[chris hofmann]

thanks

Comment 31

[David :Bienvenu]

fix checked in

Comment 32

[lohphat]

FYI, the filter get's disabled so even after you're running a fixed build, you
have to manually reactivate the filter.