76157 - Give proper error message on non-numeric sortkey.

Status: RESOLVED FIXED

(Bugzilla :: Administration, task, P2)

Description

[Matthew Tuck [:CodeMachine]]

If I try to add or edit a milestone such that the sortkey is not a number, I get
an error like:

INSERT INTO milestones ( value, product, sortkey ) VALUES ( 'A','TestProduct',
K): Unknown column 'K' in 'field list' at
globals.pl line 173. 

This should check the sortkey is a number and give a reasonable error message if
not.

This might be a security problem but I can't figure out how to might make it
one.

Comment 1

[Andreas Franke (gone)]

-> Bugzilla product, Administration component, reassigning.

Comment 2

[Matthew Tuck [:CodeMachine]]

I'll take this one.

Comment 3

[Dave Miller [:justdave]]

We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.

Comment 4

[Vlad Dascalu]

Attachment: Checks for a valid sortkey.

Comment 5

[Christian Reis]

Comment on attachment 131330 [details] [diff] [review]
Checks for a valid sortkey.

If you move your check to before the TestMilestone() clause there, r=kiko.

Reason is, this is a relatively simple check, and there's no reason to check it
against $product if it's got invalid characters in it.

(BTW: welcome back, I was wondering when you'd show up ;-)

Comment 6

[Vlad Dascalu]

Attachment: Moving code as kiko suggested.

I was mostly around but I have been quiet lately. :)

Comment 7

[Vlad Dascalu]

<-- me (patch writer)

Comment 8

[Vlad Dascalu]

Comment on attachment 131420 [details] [diff] [review]
Moving code as kiko suggested.

Not sure if I can carry over your r+ on this new version.

Comment 9

[Christian Reis]

Comment on attachment 131420 [details] [diff] [review]
Moving code as kiko suggested.

Looks fine, r=kiko

Comment 10

[Dave Miller [:justdave]]

It'd be nice if we could ThrowUserError, but seeing as how the whole thing
hasn't been templatized yet, I assume we'll pick that up when we templatize it.

a=justdave

Comment 11

[Dave Miller [:justdave]]

Checking in editmilestones.cgi;
/cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v  <--  editmilestones.cgi
new revision: 1.17; previous revision: 1.16
done