Bugzilla::Object->check should trim and be more accurate about what's "empty"

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 3.6
Bug Flags:
approval +

Details

Attachments

(1 attachment)

v1
2.04 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Assignee)

Description

8 years ago
Bugzilla::Object->check currently calls trim() to check if the input value is "empty", but it doesn't pass the trimmed value to new, which it should be doing (as check() is our primary input validator, and we should be trimming values on input). Also, currently it thinks that "0" is an invalid name for an object, even though it's a perfectly valid string.
(Assignee)

Comment 1

8 years ago
Created attachment 407883 [details] [diff] [review]
v1

This does several things:

1) It makes the trimmed get passed to _init, which I believe we used to do before we allowed check() to take ids.

2) It makes "0" a valid id and name for check(), which means that if you pass 0 as an id, you'll get a much clearer "there is no (blah) with the id 0" instead of just "you must select/enter a (blah)".

3) I made detaint_natural and detaint_signed use int() on their return values, because that makes Perl treat the object entirely like an integer and not like a string, which could be important in certain DBDs that optimize bound values when they're integers (and also simplifies detaint_signed in any case). This also means that "000" gets detainted to "0", which is more accurate. Same for "001"--it gets detainted to "1".
Assignee: general → mkanat
Status: NEW → ASSIGNED
Attachment #407883 - Flags: review?(LpSolit)

Comment 2

8 years ago
Comment on attachment 407883 [details] [diff] [review]
v1

Looks good. r=LpSolit
Attachment #407883 - Flags: review?(LpSolit) → review+

Updated

8 years ago
Flags: approval+
(Assignee)

Comment 3

8 years ago
Checking in Bugzilla/Object.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Object.pm,v  <--  Object.pm
new revision: 1.37; previous revision: 1.36
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.94; previous revision: 1.93
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.