Emails with UTF-8 in the body cause email sending to die

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Email Notifications
--
major
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

({regression})

3.1.2
Bugzilla 3.2
regression
Bug Flags:
approval +
blocking3.1.3 +

Details

Attachments

(3 attachments)

(Assignee)

Description

11 years ago
Created attachment 290158 [details] [diff] [review]
v1

Right now if your realname or a comment has a UTF-8 character in it, email sending dies with a "wide character in subroutine" error. This is because encoding_set in Email::MIME::Modifier only works with bytes--it doesn't work with strings that have the utf8 bit set.

LpSolit, you know this patch works if your email from bugzilla.everythingsolved.com on bug 240 came through OK.
Attachment #290158 - Flags: review?(LpSolit)

Comment 1

11 years ago
Comment on attachment 290158 [details] [diff] [review]
v1

Marc or Dave, could you review this one? I mostly understand what Max tries to do, but I don't know the internals of Email::MIME well enough to say if this fix is the right thing to do or not.
Attachment #290158 - Flags: review?(wurblzap)
Attachment #290158 - Flags: review?(justdave)
Attachment #290158 - Flags: review?(LpSolit)

Comment 2

11 years ago
this works for the error, i think.
but it cause another error (garbaged strings???) on bugmail.

Comment 3

11 years ago
(In reply to comment #2)
> this works for the error, i think.
> but it cause another error (garbaged strings???) on bugmail.

So be sure to validate the output when reviewing this patch. See also bug 405444.

Comment 4

11 years ago
Pushing this bug in our radar. Definitely a blocker IMO.
Flags: blocking3.1.3?
(Assignee)

Updated

11 years ago
Flags: blocking3.1.3? → blocking3.1.3+
(Assignee)

Comment 5

11 years ago
(In reply to comment #3)
> So be sure to validate the output when reviewing this patch. See also bug
> 405444.

  The email in bug 405444 looks fine to me. That's what quoted-printable encoding usually looks like.
Comment on attachment 290158 [details] [diff] [review]
v1

r=Wurblzap, although I'm a little edgy with $part methods because of $part->charset() not working (see bug 311563); hoping that no further trouble will come from this.
Attachment #290158 - Flags: review?(wurblzap) → review+
(Assignee)

Comment 7

11 years ago
(In reply to comment #6)
> (From update of attachment 290158 [details] [diff] [review])
> r=Wurblzap, although I'm a little edgy with $part methods because of
> $part->charset() not working (see bug 311563); hoping that no further trouble
> will come from this.

  At the worst, it won't break anything any differently than it was already broken. This actually just reverts behavior to exactly what it was before we checked in the utf8-bit patch.
Flags: approval+

Comment 8

11 years ago
(In reply to comment #5)
> > So be sure to validate the output when reviewing this patch. See also bug
> > 405444.
> 
>   The email in bug 405444 looks fine to me. That's what quoted-printable
> encoding usually looks like.

i think you should decode 'q-encode' first, and look into the string.
q-encoded 'garbaged utf-8 string' might always be a good q-encode, i think.

Comment 9

11 years ago
Created attachment 290359 [details]
sample mail text for comment #5

as requested on irc. :)
it seems that bug 405444 is not related on this but on FormatDouble/FormatTriple function in Bugzilla/BugMail.pm.
(Assignee)

Comment 11

11 years ago
We should just sent 8-bit mail. There's no reason not to, that I can think of, now that we have the bits in our strings set correctly, right?
(In reply to comment #11)
> We should just sent 8-bit mail. There's no reason not to, that I can think of,
> now that we have the bits in our strings set correctly, right?

agree with this.

to add another word,, using q-encode for mail body, the size of e-mail will increase for three times. it's really waste :(
# one 3 bytes utf-8 char => =xx=xx=xx style
Keywords: regression

Comment 13

11 years ago
Is this patch ready for checkin?
(Assignee)

Comment 14

11 years ago
(In reply to comment #13)
> Is this patch ready for checkin?

  Well, I'm thinking actually that I should fix it a completely different way, after talking with himorin. That is, why not just send raw UTF-8? It works with all modern mail readers and MTAs.
Flags: approval+
I like Max's idea.  Most MTAs who care will translate it to 7-bit encoded for us, and they're better at it than we are.

Comment 16

11 years ago
I met this issue today when installing chinese UI on bugzilla . 
instead of fixing bugzilla, I chose to update Email::MIME::Encoding module.
sub codec {
# ...omitted
 $sub->(utf8::is_utf8($msg)?Encode::encode('utf-8',$msg):$msg);
}
don't know if any side effect, it works just ok for me.
(Assignee)

Comment 17

11 years ago
Okay, right now sending 8bit mail causes Email::Send::Sendmail to throw a warning, though messages send fine.

I patched Email::Send::Sendmail and submitted the patch, and it will probably make its way into the next release of Email::Send:

  http://rt.cpan.org/Public/Bug/Display.html?id=31456
(Assignee)

Comment 18

11 years ago
Created attachment 292912 [details] [diff] [review]
Send as 8bit, v1

Okay, here's the simple patch that makes us send 8bit-mail. Much simpler than doing the encoding.
(Assignee)

Updated

11 years ago
Attachment #292912 - Flags: review?(wurblzap)
(Assignee)

Updated

11 years ago
Attachment #290158 - Flags: review?(justdave)
(In reply to comment #18)
> Created an attachment (id=292912) [details]
> Send as 8bit, v1
> 
> Okay, here's the simple patch that makes us send 8bit-mail. Much simpler than
> doing the encoding.

this patch seems working well.
(Assignee)

Comment 20

11 years ago
Okay, I'm going to check in the 7-bit patch for now, because of the Email::Send bug. Throwing a warning for every mail sent would seriously slow down Bugzilla. (Apache throttles warning rates.)
(Assignee)

Comment 21

11 years ago
Checking in Bugzilla/Mailer.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Mailer.pm,v  <--  Mailer.pm
new revision: 1.16; previous revision: 1.15
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: approval+
Resolution: --- → FIXED

Comment 22

11 years ago
Why is this bug marked as fixed as there is still a pending request for review?
(Assignee)

Comment 23

11 years ago
(In reply to comment #22)
> Why is this bug marked as fixed as there is still a pending request for review?

  Because we didn't check in that patch, but we still might at some point in the future.
Comment on attachment 292912 [details] [diff] [review]
Send as 8bit, v1

Frédéric is right. If at all, then this should be done in a separate bug.
Attachment #292912 - Flags: review?(wurblzap)
You need to log in before you can comment on or make changes to this bug.