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

()

--
major
RESOLVED FIXED
15 years ago
12 years ago

People

(Reporter: kiko, Assigned: LpSolit)

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

15 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

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

Comment 2

13 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?
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

13 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

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

Updated

13 years ago
Blocks: 223420
(Assignee)

Updated

13 years ago
Blocks: 224148
(Assignee)

Updated

13 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

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

Comment 9

13 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

13 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

13 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

13 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

13 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

13 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: 13 years ago
QA Contact: mattyt-bugzilla → default-qa
Resolution: --- → FIXED
(Assignee)

Updated

13 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.