113646 - midair when changing assignee gives error

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Bradley Baetz (:bbaetz)]

I midaired while changing the assignee on a couple of the smoketest blockers
this morning, and went to continue (they were just comment changes)

I got:

unknown action '[e-mail]'

Comment 1

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

Is the error message you reported capitalized correctly?  I was unable to find
an instance of the phrase "unknown action" in the Bugzilla code.

Comment 2

[Bradley Baetz (:bbaetz)]

No, probably not. the machine I was on kept crashing, and teh keyboard had
broken vowel keys...

Trying locally, we get:

<input type=hidden name="knob" value="bbaetz+foo@localhost">

in the midair page, and so I get "Unknown action bbaetz+foo@localhost!" after
trying to continue anyway

Comment 3

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

Found the problem.

Comment 4

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

Attachment: patch v1: rewrites trim not to use $_ global variable

The problem with the code is that the trim() function in globals.pl uses the
global variable $_ to store the value being trimmed.  Normally this won't cause
problems, but in this situation the Perl interpreter is already using $_ as the
value of $::FORM{'knob'} in the switch that starts on line 672 of
process_bug.cgi.

I'm not sure exactly why this causes problems, but I suspect that Perl copies
the value of $::FORM{'knob'} into $_ by reference rather than by value, so the
value of $::FORM{'knob'} gets changed to the trimmed value of
$::FORM{'assigned_to'} when the latter variable gets trimmed on line 708.

Using $_ as a temporary variable in trim() seems like a dangerous thing to do
in general, so the patch changes trim() to use a local variable instead.

Comment 5

[Bradley Baetz (:bbaetz)]

Comment on attachment 60596 [details] [diff] [review]
patch v1: rewrites trim not to use $_ global variable

r=bbaetz

Comment 6

[Bradley Baetz (:bbaetz)]

forgot to mention: I only gave this a very very quick test - 2nd reviewer needs
to do a real test.

Comment 7

[Dave Miller [:justdave]]

Comment on attachment 60596 [details] [diff] [review]
patch v1: rewrites trim not to use $_ global variable

r= justdave
it works.

Comment 8

[Zach Lipton [:zach]]

Fix checked in