46431 - Sig delimiter malformed for quoted printable

Status: VERIFIED FIXED

(Core :: DOM: Serializers, defect, P3)

Description

[Ben Bucksch (:BenB)]

Reproduction:
1. Check Prefs|Mailnews|Composer|quoted-printable (i.e. not "as-is")
2. Set your account preffs to use a plaintext signature with only one normal
line, e.g. "foo", as content
3. Compose a new msg using the HTML composer with 8bit-chars, e.g. "дьц".
4. Send
5. Look at the source

Actual result:
The sig reads
<example>
-- =

foo
</example>.

Expected result:
The sig reads
<example>
-- 
foo
</example> (with a space after "--").

Relevance:
This is kind-of a "standards-conformance" bug. IIRC, the sig delimiter is
somewhere standardized in a news RFC. If it is only slightly wrong, software
(including Mozilla) won't recognize it anymore. nsbeta3 nomination for this
reason.

Additional comments:
Not sure, if this is a HTML->TXT, Editor or libmime bug. Guessing HTML->TXT
first.

Comment 1

[Akkana Peck]

I'm definitely not the one adding the "=".  Guessing libmime.

Comment 2

[rhp (gone)]

So the problem is looking at quoted printable source and seeing the "="?

Comment 3

[Daniel Bratell]

Looks like a quoted printable trailing space. If this is a bug, it's probably in 
the quoted printable encoder or in the use of that encoder.

The same problem exists with format=flowed mails which is why RFC 2646 
specifically says that QP shouldn't be used.

Comment 4

[Ben Bucksch (:BenB)]

akk, sure :). This is quoted-pritable for a flowed line, just like the trailing
space in format=flowed. So, anything thinks, the sig delimiter were a flowed
line.

Not very surprisingly, Debug|OutputText in the editor doesn't show the "=".
Could this have the same source as bug 33492: an odd data path during send?

Rich, the problem is that is it there in the first place. It shouldn't. "-- " is
not a flowed line. I have no idea why it thinks it is. I guess, we're using the
libmime TXT->HTML conversion (precisely mimetpfl) somewhere during the send
process (and we shouldn't of course).

Comment 5

[Ben Bucksch (:BenB)]

> I guess, we're using the
> libmime TXT->HTML conversion (precisely mimetpfl) somewhere during the send
> process (and we shouldn't of course).

Wait, even mimetpfl knows that "-- " is a fixed line, right? Odd.

Comment 6

[rhp (gone)]

If you are looking at quoted printable source, you will see the "=" as the 
delimiter at the end of the line.

I'm still not sure I see the bug here?

- rhp

Comment 7

[Ben Bucksch (:BenB)]

Rich, sorry, I don't see your problem. "-- " must be one its own line *in the
msg source*. It isn't. That's a bug.

Turns out we do that for every line. Type a paragraph > 80 chars (incl. 8bit
chars). Send. View Source. You'll see e.g.
<example>
tzuitzuizui=FC=E4=FC=F6=E4ertzertz zzzzzzzzzzzz zzzzzzzzzzzz =

zzzzzrtzzzzzzzzzzzzzzz zzzzzzzzzzzzzzzzz zzzzzz zzzzzzzzzzzzzz =

zzzzzzzzzzzzzz zzrtzzzzzzzz zzzzzzzzzzzz

-- =

<http://www.bucksch.org>
</example>

Comment 8

[rhp (gone)]

Well, I can look at this if I have time, but this doesn't really feel beta3 to 
me.

- rhp

Comment 9

[Daniel Bratell]

But all this quoted printable things should be removed from the mail before it 
reaches anything/anyone in the mail program, right? Isn't that done now? That QP 
sucks, we already now, but at least it's not the default anymore.

Comment 10

[rhp (gone)]

Quoted printable data lives in the file until rendered. It should not be 
removed until the rendering stage. Yes, this stuff should not be seen by the 
user (unless they want to look at the source of the message). This is why I say 
this isn't a beta3 type issue. 

- rhp

Comment 11

[Ben Bucksch (:BenB)]

> Yes, this stuff should not be seen by the 
> user (unless they want to look at the source of the message).

But it breaks the sig. And the user (the recipient in this case) will surely see
*that*. In Mozilla, we render thesig differently and remove it during reply. All
this breaks. And not only for us, for *all* recipients. That's why this is a
standards-conformance bug.

Comment 12

[Daniel Bratell]

But it shouldn't. For instance, I think the quoted printable is removed before 
the text is coming to mimetpfl.cpp. Has you checked what mimetpfl.cpp sees?

Comment 13

[Ben Bucksch (:BenB)]

No, I didn't. But if you reply to such a msg, the sig is not removed.

(Completely OFFTOPIC: Daniel, did you get my msg to idasys? My mail to your
lysator account bounces, because the server doesn't accept mail directly from
dynamic IP addresses. :-( )

Comment 14

[Daniel Bratell]

The quoted printable encoder treats trailing spaces wrongly. See 
<http://lxr.mozilla.org/seamonkey/source/mailnews/mime/src/mimeenc.cpp#888>. A 
trailing space shouldn't (generally at least) be encoded as a soft line break, 
but with =20 which is the code for SPACE. If it is treated as a soft line break 
by the receiver we could very well get the behaviour that Ben describes.

Comment 15

[Daniel Bratell]

Wait, I see what it does now. I trailing space followed by a soft line break 
followed by an empty line should theoretically cause the correct string when 
decoded. But I still think that we should encode the space as =20 (or =09 if 
it's a tab). That means back up one step in the out string and overwrite the 
SPACE (TAB) with the code. 

Rich, is there special reason to do what we do right now?

Comment 16

[rhp (gone)]

Probably no special reason aside from the fact that its been working for years 
so I'm always very cautious making changes to code like this as we approach the 
next beta.

- rhp

Comment 17

[Ben Bucksch (:BenB)]

> its been working for years

But then, we had no format=flowed and caused unreadable mails. Not everybody has
a QP-aware mailer.

> a soft line break followed by an empty line should theoretically cause the
> correct string when decoded.

This makes assumption about the idea of a paragraph (that it equals a line) in
the recieving mailers, which we must not make. If the assumption is wrong, the
sig won't be recognized.

Anyway, *we* break. We don't recognize the sig we put out.

Comment 18

[rhp (gone)]

> > its been working for years

> But then, we had no format=flowed and caused unreadable mails. Not everybody 
> has a QP-aware mailer.

The code is at:
    http://lxr.mozilla.org/seamonkey/source/mailnews/mime/src/mimeenc.cpp#888

Feel free to hack away.

- rhp

Comment 19

[Ben Bucksch (:BenB)]

Over to Daniel, you said he might be looking at it anyway.

Comment 20

[Ben Bucksch (:BenB)]

s/you/who

Comment 21

[Daniel Bratell]

What? I did that before today. See my comments as of 2000-07-26 02:51 and 
2000-07-26 02:57. I didn't attach the patch though so here it comes. I won't do 
any extensive testing of how good the patch is though. I works for me. I get sig 
removal when replying.

Index: src/mimeenc.cpp
===================================================================
RCS file: /cvsroot/mozilla/mailnews/mime/src/mimeenc.cpp,v
retrieving revision 1.8
diff -u -r1.8 mimeenc.cpp
--- src/mimeenc.cpp     1999/11/06 03:32:39     1.8
+++ src/mimeenc.cpp     2000/07/26 18:45:30
@@ -887,15 +887,17 @@
        {
          if (*in == CR || *in == LF)
                {
-                 /* Whitespace cannot be allowed to occur at the end of the lin
e.
-                        So we encode " \n" as " =\n\n", that is, the whitespace
, a
-                        soft line break, and then a hard line break.
-                  */
-                 if (white)
-                       {
-                         *out++ = '=';
-                         *out++ = CR;
-                         *out++ = LF;
+          /* Whitespace cannot be allowed to occur at the end of
+             the line, so we backup and replace the whitespace with
+             it's code.
+          */
+          if (white)
+            {
+              out--;
+              char whitespace_char = *out;
+              *out++ = '=';
+              *out++ = hexdigits[whitespace_char >> 4];
+              *out++ = hexdigits[whitespace_char & 0xF];
                        }

                  /* Now write out the newline. */


Back to Ben. You may play with this and check it in if Rich thinks it's good. 
The M17/nsbeta2 branch is today so I guess the tree will be open to a little 
more risky checkins again tonight or tomorrow.

Comment 22

[rhp (gone)]

Fine with me guys...just make sure it doesn't go into the tree until after we 
branch.

- rhp

Comment 23

[Ben Bucksch (:BenB)]

Daniel, then, this was a misunderstanding.

Just because I report a bug doesn't mean I am willing to fix it. I have too many
bugs on my plate already. Back to Rich.

Comment 24

[rhp (gone)]

Guys, this is getting stupid. I'll take care of this (THIS TIME), but in the 
future, can we have less crap filling my inbox (i.e. just email each other)..

- rhp

Comment 25

[Ben Bucksch (:BenB)]

Rich, I have problems to follow you. Shouldn't I file bugs?

Comment 26

[rhp (gone)]

Filing bugs is ok. But bouncing them back and forth, marking them as beta 
stoppers and commenting on every statement I make is not productive at this 
point in the process. I don't need all the distractions and if you have time to 
do all of the bugzilla work, you would have as much time as me to apply the 
patch and test it.

- rhp

Comment 27

[rhp (gone)]

I will get this into the tree when it opens for beta 3.

- rhp

Comment 28

[rhp (gone)]

per selmer and mail/news PDT.

Comment 29

[rhp (gone)]

Just checked in a fix.

- rhp

Comment 30

[rhp (gone)]

one more time...

Comment 31

[sujay]

marking verified.