103658 - if a filter rule contains an odd number of quotes, Mozilla is unable to parse it

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect)

Description

[Alexey Chernyak]

Build: 2001100503

Steps to reproduce:
1. Create a filter rule with odd number of quotes and a few more rules behind
it. For example:
condition="OR (subject,contains,\\") OR (subject,contains,moo) OR
(subject,contains,foo) OR (subject,contains,\"moo ) foo\")"
2. Quit Mozilla.
3. Open Mozilla and check how it has read the rules from the file.

Actual Results: Complete mess, see for yourself.
Expected Results: Have the rules the same way you entered them.

Even worse, if you create a rule on a quote escape sequence like this:
condition="OR (subject,contains,\\\")"
on load, *all* the filters (not rules) that follow will disappear.

I understand this is an issue of a bad syntax design and changing syntax at this
stage is unacceptable. However, I found a quirk that might help us fix all this
without changing syntax. When Mozilla saves a rule with quotes in it, it escapes
the quotes twice like this \\". When it encloses a rule in quotes like in case
when they contain ')' character, then the outside quotes are escaped once like
this \" (see
http://lxr.mozilla.org/mozilla/source/mailnews/base/search/src/nsMsgSearchTerm.cpp#339
). And the quotes that contain all rules are not escaped at all.
So you can apply simple logic in here: If a double escaped quote is encountered
\\" - that means that the quote is contained inside the rule string.
This is a hack, but since we can't change the syntax at this stage, the only
alternative we have is forbid quotes inside rules. But that would mean
introducing a limitation on a great (we'd like to think) product, which would
suck, and breaking backward compatibility which would suck even more.

Comment 1

[David :Bienvenu]

Your analysis looks correct to me.  It's unfortunate that some quotes are double
escaped. We'd need to make sure that only quotes are escaped (e.g., what happens
with '/' ? Are any other characters escaped?) I think not, but we should make sure.

Comment 2

[David A. Cobb]

Re: the last comment -
Why not use the same sort or syntax used in some programming languages that
'\' escapes [makes literal] whatever follows.  Personally, I see no good reason
for the quotes that enclose the whole condition statement, but what do I know.

While we're at it, there seems no _good_ reason for including quotes around
custom headers - they all have the same one-word form.

If the interior '\"'s or '\)'s or '\\'s and any other troublesome character is
escaped there is no reason to care whether the un-escaped, un-quoted string has
an even or odd number of anything.

Comment 3

[David A. Cobb]

Bug#104799 suggests the comma must also be escaped.  I would nominate one of
these as duplicate

Comment 4

[Dr. Seltsam]

This bug can be reproduced with an even number of quotes, too:
1. open filter ui
2. create new filter rule: sender contains "(my name plus)_xyz"@hotmail.com
3. save rule and filter
4. receive new mails (a restart of Mozilla is not necessary)
5. open the filter and the condition of the rule now is: "(my name plus
6. all following filter rules were deleted

reproducable with Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.1) Gecko/20020826

Comment 5

[David A. Cobb]

I've also found another case that gets messed up:

One of the Acrobat lists at Adobe.com sends mail with the following headers --

From: <Acrobat Answers to Frequently Asked Questions (FAQ)>
To: <Acrobat Answers to Frequently Asked Questions (FAQ)>
Subject: re: Launching Acrobat or Reader
Message-Id: <[e-mail]2eafNXanI>
Mime-Version: 1.0

If I clip the From or To and attempt to use them, (1) a search doesn't find
anything, and (2) I can store the filter but, when it is stored the filter rule
is set to match <empty-string> so EVERY piece of mail gets a hit on the damaged
filter and gets moved where it doesn't belong.

Looks as though the parens are a significant part of the problem.

Comment 6

[Rob North]

Just came across this bug. Been driving me up the wall!

I think this should be a major bug, as it causes data loss:
You loose all filters after the filter with quote.

Not only that, but while mozilla is running you can go back to the filter dialogs,
and see the filter. You can even, in the latest builds run the filter on any
folder and it will work correctly.

Only on close does this become a problem.

Hack it, disable it, or put it in the release notes, but please do something
about it!

Comment 7

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

mass re-assign.

Comment 8

[Stefan Borggraefe]

This bug also happens with Thunderbird 20030808 on Linux.

Comment 9

[Vittorio Bertola]

This bug is REALLY nasty for the following reason: I entered a filter rule that
ended by a quote, and it worked fine until I closed Mozilla. But when I opened
Mozilla the following time, the filter was there, but was empty... so it was
being applied to ALL messages coming in from my mailbox. If by chance it is a
deletion or mark read filter, you may imagine what this may mean :-(

So please do something about it... at least, you could for the time being forbid
quotes inside filter rules, by making the filter editor reject them. At least
you wouldn't cause unexpected data loss to those using Mozilla as primary mail
program...

Thanks :)

Comment 10

[Ian Konen]

I see the same thing as comment #5 with the < delimiter.

The filter doesn't actually work during the same session, but the settings seem
to be retained when the message filters dialog is reopened.  Upon quitting
Mozilla and restarting, the filter conditions have dissappeared entirely leaving
a "match any of the following" with no conditions, which matches all messages.  

Comment 11

[Alexey Chernyak]

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

Comment 12

[Mike Cowperthwaite]

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

Comment 13

[Magnus Melin [:mkmelin]]

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

Comment 14

[Magnus Melin [:mkmelin]]

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

Comment 15

[Guy Alfandary]

5 years are more than enough to fix such a trivial bug,
or should I do it myself?

Comment 16

[Håkan Waara]

(In reply to comment #15)
> 5 years are more than enough to fix such a trivial bug,
> or should I do it myself?

I bet people would appreciate if you do it!

Comment 17

[Guy Alfandary]

Attachment: Proposed fix

(In reply to comment #16)
> (In reply to comment #15)
> I bet people would appreciate if you do it!
I doubt that they would, in fact (from experience) I expect
quite the opposite (no good deed goes unpunished and all
that). Oh well, forever the optimist I attach a proposed
fix, please don't hold that against me.

Comment 18

[Mike Cowperthwaite]

You need to ask for review on that patch in order for it to move forward.

Comment 19

[Guy Alfandary]

(In reply to comment #18)
> You need to ask for review on that patch in order for it to move forward.
> 
Hopefully this patch would fix your problem as it did mine,
however I have no official capacity here. This was open
for 5 years and would likely stay so 5 more :-(

Comment 20

[Magnus Melin [:mkmelin]]

To request review for the patch, choose the Edit action for it and set the 'r?' flag to a reviewer, e.g. [e-mail] or [e-mail]

Comment 21

[Guy Alfandary]

(In reply to comment #20)
> To request review for the patch, choose the Edit action for it and set the 'r?'
> flag to a reviewer, e.g. [e-mail] or [e-mail]
> 
You live and learn. At any rate, you live.
Thanks.

Comment 22

[David :Bienvenu]

Comment on attachment 241707 [details] [diff] [review]
Proposed fix

thx for the patch! it looks correct to me... I haven't had a chance to verify it yet, though.

Comment 23

[Scott MacGregor]

Comment on attachment 241707 [details] [diff] [review]
Proposed fix

thanks for the patch!

Comment 24

[Scott MacGregor]

You might be able to talk Gavin into checking this in for you. 

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Checked in to the trunk.
mozilla/extensions/irc/xul/content/chatzilla.xul 	1.67

Comment 26

[Mike Cowperthwaite]

Comment on attachment 241707 [details] [diff] [review]
Proposed fix

(In reply to comment #25)
> Checked in to the trunk.
> mozilla/extensions/irc/xul/content/chatzilla.xul        1.67

seamonkey/source/mailnews/base/search/src/nsMsgFilterList.cpp  1.101  

would be the correct filename and version.  :)

Comment 27

[Scott MacGregor]

Comment on attachment 241707 [details] [diff] [review]
Proposed fix

this has baked on the trunk for quite a while.

Comment 28

[Carsten Book [:Tomcat]]

Attachment: error after restart with the steps from comment#0

the steps to reproduce from comment #4 are working fine for me on Thunderbird 2 RC 1. 

When i try the steps from comment#0 with the rules 

"OR (subject,contains,\\") OR (subject,contains,moo) OR
(subject,contains,foo) OR (subject,contains,\"moo ) foo\")"

I get a error Message on Thunderbird restart (see screenshot), its this expected ?

Comment 29

[Mike Cowperthwaite]

It would be helpful for the steps-to-reproduce if you stated exactly what you were typing into the UI, rather than what the file contains; I really can't tell what text in the UI corresponds to: 
[...] (subject,contains,\"moo ) foo\")"

It seems that whatever you type is copied verbatim into the file -- that is, if you type three backslashes, three backslashes are what gets stored.  On read-back, those are misparsed for display; so there is at least one bug lurking here.

If you type an unmatched quote, it gets a backslash of its own.

I *think* that the clause quoted above implies this text in the UI (including quotes):
   "moo ) foo"
but when I type that in, the msgFilterRules.dat contains:
   (subject,contains,\"\\"moo ) foo\\"\")
which is read back in without an error.  (TB 2pre-0324)