If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Remove %FORM from editkeywords.cgi

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
14 years ago
5 years ago

People

(Reporter: Vlad Dascalu, Assigned: Vlad Dascalu)

Tracking

2.17.6
Bugzilla 2.18
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

2.04 KB, patch
Christian Reis
: review+
Details | Diff | Splinter Review
(Assignee)

Description

14 years ago
Use $cgi->param in editkeywords.cgi.
(Assignee)

Comment 1

14 years ago
Created attachment 141739 [details] [diff] [review]
Version 1
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.18
(Assignee)

Updated

14 years ago
Attachment #141739 - Flags: review?(kiko)

Comment 2

14 years ago
Comment on attachment 141739 [details] [diff] [review]
Version 1

>Index: >@@ -173,7 +173,10 @@
> #
> 
> if ($action eq 'edit') {
>-    my $id  = trim($::FORM{id} || 0);
>+    my $id = cgi->param('id');
>+    detaint_natural($id);
>+    $id  = trim($id || 0);

Make sure you still need the trim after detaint_natural; I'm not sure myself.
Is zero used for anything special here? If so, it would be nice to add a
comment...
Attachment #141739 - Flags: review?(kiko) → review+
Comment on attachment 141739 [details] [diff] [review]
Version 1

>@@ -173,7 +173,10 @@
>-    my $id  = trim($::FORM{id} || 0);
>+    my $id = cgi->param('id');
>+    detaint_natural($id);
>+    $id  = trim($id || 0);

kiko's got a point... this won't work.	If the value had spaces around it and
thus needs to be trimmed, it's going to fail the detaint_natural.  Need to trim
it first before calling detaint_natural on it.
Attachment #141739 - Flags: review-
(Assignee)

Comment 4

14 years ago
It used to get trimmed after detaining it in the old code. But good point, 
*nods*.
(Assignee)

Comment 5

14 years ago
Created attachment 142619 [details] [diff] [review]
Version 2

0 is not a special case, so I removed that as well.
Attachment #141739 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #142619 - Flags: review?(kiko)

Updated

14 years ago
Attachment #142619 - Flags: review?(kiko) → review+
(Assignee)

Updated

14 years ago
Flags: approval?
Flags: approval? → approval+
(Assignee)

Comment 6

14 years ago
Checking in editkeywords.cgi;
/cvsroot/mozilla/webtools/bugzilla/editkeywords.cgi,v  <--  editkeywords.cgi
new revision: 1.20; previous revision: 1.19
done
(Assignee)

Updated

14 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.