266147 - Internal error when Flag::notify() ends up with an invalid or empty CC: list and no requestee in restricted bugs

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Christian Reis]

When Attachment::notify() ends up with an invalid or empty CC: list (because of
the isprivate/restricted flags) and there is no requestee, we try to sendmail()
and of course, an email with no To: and no CC: list won't run. We get an
internal error and the server logs will say:

[Tue Oct 26 14:46:01 2004] [error] [client 192.168.99.6] malformed header from
script. Bad header=No recipient addresses found i:
/home/kiko/www/mybugzilla/attachment.cgi

This is sendmail returning us a bad message and us displaying it to the end-user.

The easiest fix is probably band-aiding the sendmail call to not blow up when we
have no To: and no CC:.

Comment 1

[Joel Peshkin]

Yep.  Not just sendmail,  any MTA

Comment 2

[Frédéric Buclin]

Attachment::notify() doesn't exist. Updating the summary accordingly.

Comment 3

[Marc Schumann [:Wurblzap]]

Is this related to bug 284590? Will it perhaps even fix bug 284590?

Comment 4

[Max Kanat-Alexander]

What's the earliest version affected? (I'm just guessing 2.18 and putting that in the version field.)

I don't think this is something we need to fix in 2.20, and I don't think it's something I'd block the release on, but it is a good idea to get all the Internal Errors fixed. It's an bad error, but I haven't heard that many people report it.

Comment 5

[Teemu Mannermaa (:wicked)]

(In reply to comment #4)
> I don't think this is something we need to fix in 2.20, and I don't think it's
> something I'd block the release on, but it is a good idea to get all the

This decision makes an interesting situation for me. This bug is somewhat blocking bug 284590 that IS marked blocking the release.. I still think we need to fix this bug before we know if 284590 is still a problem. My bet is that it isn't but who knows.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

I needed to rewrite Flag.pm a bit to correctly check whether the To: and CC: fields were both emtpy or not. As a cool side-effect, this patch also fixes bug 223420, bug 224148 and bug 251869. :)

myk, you will note that Flag::notify() now returns if there is nobody to notify.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 205353 [details] [diff] [review]
patch, v1

Looks good, r=myk

Comment 8

[Frédéric Buclin]

ok, it needs a backport for 2.20 to fix a small bitrot.

Comment 9

[Frédéric Buclin]

Attachment: patch for tip, v2

I undo the patch from bug 256762 which is incorrect here (flag.addressee.email already has the emailsuffix appended, see Bugzilla::User::email). Moreover, I removed "'s request for" from the email template if there was no requester (if there is no requester, then there is no pending request).

Comment 10

[Frédéric Buclin]

Attachment: patch for 2.20, v1

backport based on patch v2 for tip.

Comment 11

[Marc Schumann [:Wurblzap]]

(In reply to comment #9)
> I undo the patch from bug 256762 which is incorrect here (flag.addressee.email
> already has the emailsuffix appended, see Bugzilla::User::email). Moreover, I

Huh? Bug 256762 should've been undone by bug 277389, shouldn't it?

Comment 12

[Frédéric Buclin]

(In reply to comment #11)
> Huh? Bug 256762 should've been undone by bug 277389, shouldn't it?

bug 277389 removed the part about the CC list. Here I remove the remaning part about the addressee. I did some tests: the template looks at User::email, not at the lines I remove here.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 205481 [details] [diff] [review]
patch for tip, v2

Good catches. r=myk

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 205482 [details] [diff] [review]
patch for 2.20, v1

Looks good; r=myk

But one note: this comment change looks wrong to me:

>-#                 Frédéric Buclin <[e-mail]>
>+#                 Frédéric Buclin <[e-mail]>

It looks like you are replacing "Frederic" with the accent aigus over both e-s with strange characters (a capital A with tilde followed by a copyright sign).  Are you switching to Unicode characters, and if so are you sure this is nonproblematic?

Comment 15

[Frédéric Buclin]

Yup, switching to UTF-8, as requested on IRC. It's working fine as several files have my name as UTF-8 already.

Comment 16

[Frédéric Buclin]

tip:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.58; previous revision: 1.57
done
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v  <--  email.txt.tmpl
new revision: 1.10; previous revision: 1.9
done


2.20:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.45.2.3; previous revision: 1.45.2.2
done
Checking in template/en/default/request/email.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/request/email.txt.tmpl,v  <--  email.txt.tmpl
new revision: 1.9.4.1; previous revision: 1.9
done