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

RESOLVED FIXED in Bugzilla 2.20

Status

()

Bugzilla
Attachments & Requests
--
major
RESOLVED FIXED
13 years ago
11 years ago

People

(Reporter: Christian Reis, Assigned: Frédéric Buclin)

Tracking

2.18
Bugzilla 2.20
Dependency tree / graph
Bug Flags:
approval +
blocking2.22 -
approval2.20 +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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

12 years ago
Yep.  Not just sendmail,  any MTA
Flags: blocking2.22?
(Assignee)

Comment 2

12 years ago
Attachment::notify() doesn't exist. Updating the summary accordingly.
Summary: Internal error when Attachment::notify() ends up with an invalid or empty CC: list and no requestee in restricted bugs → Internal error when Flag::notify() ends up with an invalid or empty CC: list and no requestee in restricted bugs
Is this related to bug 284590? Will it perhaps even fix bug 284590?

Comment 4

12 years ago
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.
Flags: blocking2.22? → blocking2.22-
OS: Linux → All
Hardware: PC → All
Version: unspecified → 2.18
(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.
(Assignee)

Comment 6

12 years ago
Created attachment 205353 [details] [diff] [review]
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.
Assignee: myk → LpSolit
Status: NEW → ASSIGNED
Attachment #205353 - Flags: review?(myk)
(Assignee)

Updated

12 years ago
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Updated

12 years ago
Blocks: 223420
(Assignee)

Updated

12 years ago
Blocks: 224148
(Assignee)

Updated

12 years ago
Blocks: 251869
Comment on attachment 205353 [details] [diff] [review]
patch, v1

Looks good, r=myk
Attachment #205353 - Flags: review?(myk) → review+
(Assignee)

Comment 8

12 years ago
ok, it needs a backport for 2.20 to fix a small bitrot.
Flags: approval?
(Assignee)

Comment 9

12 years ago
Created attachment 205481 [details] [diff] [review]
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).
Attachment #205353 - Attachment is obsolete: true
Attachment #205481 - Flags: review?(myk)
(Assignee)

Comment 10

12 years ago
Created attachment 205482 [details] [diff] [review]
patch for 2.20, v1

backport based on patch v2 for tip.
Attachment #205482 - Flags: review?(myk)
(Assignee)

Updated

12 years ago
Flags: approval?
(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?
(Assignee)

Comment 12

12 years ago
(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 on attachment 205481 [details] [diff] [review]
patch for tip, v2

Good catches. r=myk
Attachment #205481 - Flags: review?(myk) → review+
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 <LpSolit@gmail.com>
>+#                 Frédéric Buclin <LpSolit@gmail.com>

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?
Attachment #205482 - Flags: review?(myk) → review+
(Assignee)

Comment 15

12 years ago
Yup, switching to UTF-8, as requested on IRC. It's working fine as several files have my name as UTF-8 already.
Flags: approval?
Flags: approval2.20?
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval+
(Assignee)

Comment 16

12 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
QA Contact: mattyt-bugzilla → default-qa
Resolution: --- → FIXED
(Assignee)

Updated

12 years ago
Blocks: 183868
Whiteboard: [applied to b.m.o]
Whiteboard: [applied to b.m.o]
You need to log in before you can comment on or make changes to this bug.