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

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
User Interface
--
trivial
RESOLVED FIXED
15 years ago
6 years ago

People

(Reporter: Stephen Lee, Assigned: Nick Barnes)

Tracking

2.17.6
Bugzilla 2.18
Bug Flags:
approval +
blocking2.20 +
approval2.18 +
blocking2.18.2 +

Details

(Whiteboard: [wanted for 2.20])

Attachments

(1 attachment, 1 obsolete attachment)

883 bytes, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
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!
Flags: blocking2.20+
Target Milestone: --- → Bugzilla 2.20
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
Whiteboard: [wanted for 2.20]
(Assignee)

Comment 1

13 years ago
Created attachment 181954 [details] [diff] [review]
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.
Attachment #181954 - Flags: review?
(Reporter)

Comment 2

13 years ago
(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

13 years ago
Comment on attachment 181954 [details] [diff] [review]
trunk patch

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

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;";
}
Attachment #181954 - Flags: review? → review-
(Assignee)

Comment 4

13 years ago
Created attachment 182490 [details] [diff] [review]
fresh trunk patch

Amended as per your suggestion; I now have a "marked as DUPLICATE with entry in
duplicates table" test case.
Attachment #181954 - Attachment is obsolete: true
Attachment #182490 - Flags: review?

Comment 5

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

r=LpSolit
Attachment #182490 - Flags: review? → review+

Updated

13 years ago
Assignee: myk → nb+bz
Flags: approval?
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.
(Err, redhat bug 345678 would be the actual test case.)
After discussions with lpsolit on IRC, the current fix is OK, although a larger
issue may remain. a=myk
Flags: approval? → approval+
do we have this problem in 2.18 also?

Comment 10

13 years ago
(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.

Updated

13 years ago
Flags: approval2.18?
Target Milestone: Bugzilla 2.20 → Bugzilla 2.18
Flags: blocking2.18.2+
Flags: approval2.18+

Comment 11

13 years ago
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
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Flags: approval2.18?
Resolution: --- → FIXED
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>');
     }
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.