340609 - Bug->check() / ValidateBugId() should throw an error if no bug ID is passed

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[timeless]

Software error:
Undef to trick_taint at Bugzilla/Util.pm line 63
	Bugzilla::Util::trick_taint('undef') called at Bugzilla/Bug.pm line 708
	Bugzilla::Bug::bug_alias_to_id('undef') called at CGI.pl line 156
	main::ValidateBugID('undef') called at /opt/webtools/bugzilla/attachment.cgi line 829
	main::enter() called at /opt/webtools/bugzilla/attachment.cgi line 89

For help, please send mail to the webmaster ([e-mail]), giving this error message and the time and date of the error.

Comment 1

[Vlad Dascalu]

You specified in your report https://bugzilla.mozilla.org/attachment.cgi?bug_id=spelling&action=enter as URL, but actually it's "bugid" and not "bug_id".

The following URL reproduces the error:

https://bugzilla.mozilla.org/attachment.cgi?action=enter

Comment 2

[Vlad Dascalu]

This is also happening on viewall:

https://bugzilla.mozilla.org/attachment.cgi?action=viewall

(and possibly others as well)

Comment 3

[Ryszard Lach]

I have a similar problem:

Undef to trick_taint at Bugzilla/Util.pm line 66
        Bugzilla::Util::trick_taint('undef') called at
.../editcomponents.cgi line 197

My Bugzilla version is 2.22.

Comment 4

[Rèna Tim]

I have similar (if not the same) problem:

Software error:
Undef to trick_taint at Bugzilla/Util.pm line 67
	Bugzilla::Util::trick_taint('undef') called at Bugzilla/Auth/Persist/Cookie.pm line 61
	Bugzilla::Auth::Persist::Cookie::persist_login('Bugzilla::Auth::Persist::Cookie=ARRAY(0x2d5327c)', 'Bugzilla::User=HASH(0x2f09ea8)') called at Bugzilla/Auth.pm line 147
	Bugzilla::Auth::_handle_login_result('Bugzilla::Auth=ARRAY(0x2378910)', 'HASH(0x2d53bdc)', 2) called at Bugzilla/Auth.pm line 92
	Bugzilla::Auth::login('Bugzilla::Auth=ARRAY(0x2378910)', 2) called at Bugzilla.pm line 232
	Bugzilla::login('Bugzilla', 0) called at C:\Bugz\index.cgi line 40

For help, please send mail to this site's webmaster, giving this error message and the time and date of the error. 

[Thu Mar 27 11:46:53 2008] index.cgi: Undef to trick_taint at Bugzilla/Util.pm line 67 [Thu Mar 27 11:46:53 2008] index.cgi: 
Bugzilla::Util::trick_taint('undef') called at Bugzilla/Auth/Persist/Cookie.pm line 61 [Thu Mar 27 11:46:53 2008] index.cgi: 
Bugzilla::Auth::Persist::Cookie::persist_login('Bugzilla::Auth::Persist::Cookie=ARRAY(0x2d5327c)', 'Bugzilla::User=HASH(0x2f09ea8)') called at Bugzilla/Auth.pm line 147 [Thu Mar 27 11:46:53 2008] index.cgi: 
Bugzilla::Auth::_handle_login_result('Bugzilla::Auth=ARRAY(0x2378910)', 'HASH(0x2d53bdc)', 2) called at Bugzilla/Auth.pm line 92 [Thu Mar 27 11:46:53 2008] index.cgi: 
Bugzilla::Auth::login('Bugzilla::Auth=ARRAY(0x2378910)', 2) called at Bugzilla.pm line 232 [Thu Mar 27 11:46:53 2008] index.cgi: 
Bugzilla::login('Bugzilla', 0) called at C:\Bugz\index.cgi line 40 

* This is Bugzilla 3.0.3 on perl 5.8.8
* Running on WinVista Build 6000

The problem occurs straight after login.

Comment 5

[Dave Miller [:justdave]]

we've gotten a number of reports of the second error given in comment 4 from people trying to log into bmo recently.

Comment 6

[Max Kanat-Alexander]

(In reply to comment #5)
> we've gotten a number of reports of the second error given in comment 4 from
> people trying to log into bmo recently.

  That would be a different bug.

Comment 7

[Dave Miller [:justdave]]

OK, mkanat is correct, the specific error message from Bugzilla/Util.pm is on purpose, telling us that someone's calling it wrong.  So each of the places this is being found, the bug needs to be filed based on the caller in the stack.  Comments 0 through 2 (timeless) are all the same caller (or closely-enough related to live on the same bug).

Comment 3 (Ryszard) should be a separate bug referencing editcomponents.cgi line 197 (now lives at bug 453497).

Comments 4 and 5 (Réna and justdave) should be yet another, referencing Bugzilla/Auth/Persist/Cookie.pm line 61 (now lives at bug 453499)

Comment 8

[Frédéric Buclin]

Attachment: patch for tip, v1

We should immediately throw an error when no bug ID is passed to Bug->check() / ValidateBugID().

Comment 9

[Frédéric Buclin]

Attachment: patch for 3.2, v1

Same patch as for tip. Immediately throw an error if the bug ID is undefined.

Comment 10

[Olav Vitters]

Comment on attachment 339693 [details] [diff] [review]
patch for tip, v1

r=bkor by inspection

Comment 11

[Olav Vitters]

Comment on attachment 339694 [details] [diff] [review]
patch for 3.2, v1

r=bkor by inspection

Comment 12

[Frédéric Buclin]

For the record, note that the crash only occured if aliases were in use, due to bug_alias_to_id().

tip:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.261; previous revision: 1.260
done


3.2rc1:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.241.2.13; previous revision: 1.241.2.12
done

3.0.5:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.171.2.7; previous revision: 1.171.2.6
done

Comment 13

[Frédéric Buclin]

Attachment: patch for 3.0, v1

Here is the patch I used for 3.0.5. 'improper_bug_id_field_value' doesn't exist on this branch (despite what bug 221827 said), so I used 'invalid_bug_id_or_alias', which is its old name.

Comment 14

[Max Kanat-Alexander]

Comment on attachment 339693 [details] [diff] [review]
patch for tip, v1

I thought that $self->new was supposed to handle that case and return undef for $self...

Comment 15

[Frédéric Buclin]

(In reply to comment #14)
> I thought that $self->new was supposed to handle that case and return undef for
> $self...

When aliases are in use, the crash happens well before new() is called. And even when aliases are not in use, it fills the web server error log due to regexps made against undef.

Comment 16

[Max Kanat-Alexander]

(In reply to comment #15)
> When aliases are in use, the crash happens well before new() is called.

  new() is called right below the line that was added...

> And
> even when aliases are not in use, it fills the web server error log due to
> regexps made against undef.

  Okay. Well, Bugzilla::Bug->new() should be fixed, it's violating the contract of Bugzilla::Object->new().