291297 - Improve Report Comments

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Colin Ogilvie [:cso]]

Currently, report comments keeps getting spammed presumably due to spidering.

The following patch removes the JavaScript prompt, and makes a user confirm they
want to report the comment on a seperate page - this should hopefully cut down
on the spamming of reporting comments.

Comment 1

[Colin Ogilvie [:cso]]

Attachment: Patch v1

Patch as described.

Comment 2

[Colin Ogilvie [:cso]]

Attachment: Patch v2

As before, but with spaces instead of tabs.

Comment 3

[Michael Morgan [:morgamic]]

Comment on attachment 181413 [details] [diff] [review]
Patch v2

Not all inputs are properly escaped:

+	     $sql = "UPDATE `feedback` SET `flag`='YES' WHERE
`CommentID`='".$commentid."' LIMIT 1";

It would be cleaner to assign commentid and id just once, and when you do it,
escape both since they will be used in the queries.

Everything else looked fine.  Just double check that all vars are escaped and
post an updated patch.	Thanks, Colin.	:)

Comment 4

[Colin Ogilvie [:cso]]

(In reply to comment #3)
> (From update of attachment 181413 [details] [diff] [review] [edit])
> Not all inputs are properly escaped:
> 
> +	     $sql = "UPDATE `feedback` SET `flag`='YES' WHERE
> `CommentID`='".$commentid."' LIMIT 1";

Fixed, will post a new patch when I get a chance.

> It would be cleaner to assign commentid and id just once, and when you do it,
> escape both since they will be used in the queries.
> 
> Everything else looked fine.  Just double check that all vars are escaped and
> post an updated patch.	Thanks, Colin.	:)

If you mean the fact its assigned twice from $_GET and $_POST depending, then I
prefer this method to using $_REQUEST, incase a server is misconfigured.

Comment 5

[Colin Ogilvie [:cso]]

Attachment: Patch v3

v3

Comment 6

[Michael Morgan [:morgamic]]

Comment on attachment 181455 [details] [diff] [review]
Patch v3

Looks good - Thanks Colin.

Comment 7

[Michael Morgan [:morgamic]]

Pushed - will be live after next update.