1232180 - Incorrect regexp used to filter bug IDs in Bugzilla::WebService::BugUserLastVisit

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Frédéric Buclin]

In Bugzilla/WebService/BugUserLastVisit.pm, in get() and update(), the following code is used to cache bugs:

  $user->visible_bugs([grep /^[0-9]$/, @$ids])

/^[0-9]$/ will only catch bugs having only one digit. This regexp should contain [0-9]+ or \d+.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8697857 [details] [diff] [review]
patch, v1

Review of attachment 8697857 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch, but please avoid \d in new code unless you really mean all digits of all alphabets (or specify the behavior with the relevant pragma)

Comment 3

[Frédéric Buclin]

(In reply to Dylan William Hardison [:dylan] from comment #2)
> Nice catch, but please avoid \d in new code unless you really mean all
> digits of all alphabets (or specify the behavior with the relevant pragma)

\d+ is what we use everywhere in the Bugzilla code, including detaint_natural() which we use everywhere for our security checks. \d+ is safe here because if non-ASCII digits are passed and do not match any bug ID, they will simply be ignored. So IMO, this is not a valid reason to deny review.

And Bugzilla 6.0 will require Perl 5.14 which will allow us to use /a with regexps to force an ASCII comparison, so this distinction won't be relevant either.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1232180_1.patch

Updated patch that also fixes the REST version of BugUserLastVisited.

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 8698043 [details] [diff] [review]
1232180_1.patch

Review of attachment 8698043 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 6

[David Lawrence [:dkl]]

Need a patch for 5.0 as well that does not include Bugzilla/API/* 

dkl

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Attachment: 5.0.patch

Comment 8

[David Lawrence [:dkl]]

Comment on attachment 8698528 [details] [diff] [review]
5.0.patch

Review of attachment 8698528 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 9

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/bugzilla/bugzilla.git
   e247772..c7affd0  5.0 -> 5.0
To ssh://[e-mail]/bugzilla/bugzilla.git
   dee37d0..76ecb18  master -> master