bug_email.pl is broken

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Bugzilla-General
--
blocker
RESOLVED FIXED
15 years ago
5 years ago

People

(Reporter: bbaetz, Assigned: bbaetz)

Tracking

2.17
Bugzilla 2.18
x86
Linux

Details

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

Attachments

(1 attachment)

fix
791 bytes, patch
Joel Peshkin
: review+
Joel Peshkin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

15 years ago
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.
(Assignee)

Comment 1

15 years ago
Created attachment 93677 [details] [diff] [review]
fix
(Assignee)

Updated

15 years ago
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
(Assignee)

Comment 3

15 years ago
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....

Comment 4

15 years ago
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)
(Assignee)

Comment 5

15 years ago
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.

Comment 6

15 years ago
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.
Blocks: 94850

Comment 7

15 years ago
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'");
(Assignee)

Comment 8

15 years ago
jussi: Oh. I filed a separate bug on that, and ccd you

Comment 9

15 years ago
Please don't send interpolated strings to system().  Could someone create a 
patch to send both scripts into taint mode?
(Assignee)

Comment 10

15 years ago
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 11

15 years ago
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+
(Assignee)

Comment 12

15 years ago
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
Last Resolved: 15 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in 2.16.1] [fixed in 2.14.4]

Updated

12 years ago
No longer blocks: 94850
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.