Closed Bug 405362 Opened 17 years ago Closed 17 years ago

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

Categories

(Bugzilla :: Email Notifications, defect)

3.1.2
defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 3.2

People

(Reporter: mkanat, Assigned: mkanat)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached patch v1Splinter Review
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 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)
this works for the error, i think.
but it cause another error (garbaged strings???) on bugmail.
(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.
Pushing this bug in our radar. Definitely a blocker IMO.
Flags: blocking3.1.3?
Flags: blocking3.1.3? → blocking3.1.3+
(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+
(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+
(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.
as requested on irc. :)
it seems that bug 405444 is not related on this but on FormatDouble/FormatTriple function in Bugzilla/BugMail.pm.
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
Is this patch ready for checkin?
(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.
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.
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
Attached patch Send as 8bit, v1Splinter Review
Okay, here's the simple patch that makes us send 8bit-mail. Much simpler than doing the encoding.
Attachment #292912 - Flags: review?(wurblzap)
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.
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.)
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
Closed: 17 years ago
Flags: approval+
Resolution: --- → FIXED
Why is this bug marked as fixed as there is still a pending request for review?
(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.

Attachment

General

Created:
Updated:
Size: