237774 - The text "bug 0" auto-linkifies to "<missing bug number>"

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Stephen Lee]

Bit of an edge case.... caused by the fix to bug 232447.

If the text "bug 0" is entered somewhere that gets linkified, it will appear 
as "<missing bug number>".

This Description may look a little odd (on the online version) until this bug 
is fixed - I refer you to the summary!

Comment 1

[Nick Barnes]

Attachment: trunk patch

I think this patch fixes this bug without introducing any new ones, but I don't
really understand the circumstances in which GetBugLink can be passed a bogus
string as a bug number (I infer from the previous code that it could be given
an empty string, for instance).  This patch also shows the string passed as a
bug number in the resulting HTML (quoted, of course), in the case that
detaint_natural fails.

Comment 2

[Stephen Lee]

(In reply to comment #1)
> I don't
> really understand the circumstances in which GetBugLink can be passed a bogus
> string as a bug number

It gets passed an undefined value for a bug that is resolved duplicate with no 
entry in the duplicates table (for example due to database corruption or bug 
deletion). This is the circumstance in which "<missing bug number>" is wanted.

As far as I am aware it should never be passed a truly bogus string. Prior to 
bug 232447 this was a code error:
detaint_natural($bug_num) || die "GetBugLink() called with non-integer bug 
number";


Patch looks ok to me (from inspection).

Comment 3

[Frédéric Buclin]

Comment on attachment 181954 [details] [diff] [review]
trunk patch

>+    if (! defined $bug_num) {
>+        return "<missing bug number>";
>+    }
>+    my $quote_bug_num = html_quote($bug_num);
>+    detaint_natural($bug_num) || return "<invalid bug number: $quote_bug_num>";

For dupes of deleted bugs, we now have: DUPLICATE of bug <invalid bug number: >

Maybe should you write:

if (!defined($bug_num) || $bug_num eq "") {
   return "&lt;missing bug number&gt;";
}

Comment 4

[Nick Barnes]

Attachment: fresh trunk patch

Amended as per your suggestion; I now have a "marked as DUPLICATE with entry in
duplicates table" test case.

Comment 5

[Frédéric Buclin]

Comment on attachment 182490 [details] [diff] [review]
fresh trunk patch

r=LpSolit

Comment 6

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

We should display the original text when we come across a bug reference we can't
resolve.  For one thing, it's much easier for folks to figure out the problem if
they can see the faulty number.  Also, users may intentionally enter text which
looks like a bug reference but isn't or which is a bug reference to a bug in
another database, f.e. "redhat bug 123456".  We might eventually want to linkify
the latter case, but we'll never account for every case, and that's a different
bug anyway.

Comment 7

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

(Err, redhat bug 345678 would be the actual test case.)

Comment 8

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

After discussions with lpsolit on IRC, the current fix is OK, although a larger
issue may remain. a=myk

Comment 9

[Dave Miller [:justdave]]

do we have this problem in 2.18 also?

Comment 10

[Frédéric Buclin]

(In reply to comment #9)
> do we have this problem in 2.18 also?

I'm almost sure, yes. I repeat it here again to make things clear:
The only thing this patch does is to correctly display "bug zero" instead of bug
0. If the bug number does not exist in the DB, it will be displayed anyway but
not linkified. If no bug number is passed to GetBugLink(), then this function
cannot randomly choose one for you! ;)

So I definitely don't understand myk's point. If there is a problem somewhere,
it's certainly not in this function itself, but in one of the functions which
calls this one.

Comment 11

[Frédéric Buclin]

Tip:

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.324; previous revision: 1.323
done

2.18.1:

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.270.2.10; previous revision: 1.270.2.9
done

Comment 12

[Kent Rogers [:mrbball]]

This problem has reappeared.  It looks like the checkin for bug 508737 undid the
fix for this bug.

Can somebody REOPEN this bug?  As far as I can tell, the fix is trivial:

=== modified file 'Bugzilla/Template.pm'
--- Bugzilla/Template.pm	2011-08-04 19:21:36 +0000
+++ Bugzilla/Template.pm	2011-11-08 19:21:45 +0000
@@ -317,7 +317,7 @@
     my ($bug, $link_text, $options) = @_;
     my $dbh = Bugzilla->dbh;
 
-    if (!$bug) {
+    if (! defined $bug) {
         return html_quote('<missing bug number>');
     }