Closed Bug 160631 Opened 22 years ago Closed 22 years ago

bug_email.pl is broken

Categories

(Bugzilla :: Bugzilla-General, defect)

2.17
x86
Linux
defect
Not set
blocker

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: bbaetz, Assigned: bbaetz)

Details

(Whiteboard: [fixed in 2.16.1] [fixed in 2.14.4])

Attachments

(1 file)

Last week the fixes for bug_email got merged, but it appears that noone actually
tested it afterwards...

We now chdir('..') at teh top of the script, _AND_ do so before calling processmail.

This is broken on trunk, 2.16, and 2.14.3. 'oops'

Patch coming.
Attached patch fixSplinter Review
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.18
This is what we get for waiting until the last minute to do stuff like this. Who
actually did that merge?

The fix looks fine, but it would be good to get it reviewed by someone who
actually _uses_ this stuff.

Gerv
justdave, IIRC. I was on holiday at the time, and did my patch on teh assuption
that the other one wasn't going it.

As to who uses it, b.gnome.org is it, I believe. Noone has complained about this
yet, so....
The script was broken before, and the bug report was opened very late in the 
development cycle.  The security patch got in, which was the important part; I 
was scared that anyone who managed to get the script working wouldn't have seen 
it (I didn't) and started running with such a large hole.

In the meantime, let's get everything together and make this thing really rock 
(if that's possible)
Erik - if you use this, then you may as well review it. I'll then check it in
onto all the branches, although theres no guarentee that there will ever be a
2.14.4 release.
I'll take a look at it later.  I don't have too many opportunities to work on 
it, and my local copy of the script is fairly munged from exploration on the 
last bug.
I'm using an almost identical fix myself and it does work. The only difference
is that I changed the bugzilla_email_append.pl also. There is the line

system("cd .. ; ./processmail $found_id '$SenderShort'");

and it needs to be change to

system("./processmail $found_id '$SenderShort'");
jussi: Oh. I filed a separate bug on that, and ccd you
Please don't send interpolated strings to system().  Could someone create a 
patch to send both scripts into taint mode?
Erik: I've got a separate patch to fix that now, in a different bug.

The plan is to rewrite the email stuff for 2.18 - currently, its really ugly,
and broken. When the rewrite happens, it will be done in taint mode.

Note that stuff in contrib isn't supported, and may have bugs, security or
otherwise.
Comment on attachment 93677 [details] [diff] [review]
fix

This now is restored to working order, so it certainly is a change that should
go into CVS for contrib, but the bug_email still needs a lot of work.

Specifically...
1) The shebang points to the wrong perl.

2) Instructions should cover non-procmail users...
/etc/mail/aliases:
bug-tip: "| bug_email"
/etc/smrsh/bug_email:
cd BUGZILLADIR
sudo -u bugzilla BUGZILLADIR/bug_email.pl   
/etc/sudoers:
mailnull ALL=(bugzilla) NOPASSWD: BUGZILLADIR/contrib/bug_email.pl

Also, bug_email doesn't handle email sent in plan+html format well and should
really handle attachments.

None of this takes away from the fact that this patch needs to go in, though.

When the code is rewritten, it should probably also use FindBin to locate the
correct directory.  Then, it will not be necessary to cd before invoking.
Attachment #93677 - Flags: review+
Yeah - see the various meta bugs on this + related issues

checked in on trunk, 2.16, and 2.14 branches

Not that there will probably ever be a 2.14.4, but hey... (the 2.14 branch
needed to be patched with -l (ignore whitespace), because of tabs)
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in 2.16.1] [fixed in 2.14.4]
No longer blocks: bz-email-in
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: