Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
recently we had 2 admins dealing with a spammer at once, and one clicked "edit" for a comment that the other had just deleted. he got a page with invalid foreach/implode and a sql error. The attached patch checks that the selected comment exists before proceeding and just shows "Page not found" instead.
Comment | File | Size | Author |
---|---|---|---|
#11 | comment_edit_not_found_0.patch | 1.17 KB | Freso |
#9 | comment_edit_not_found.patch | 1.18 KB | Freso |
#7 | comment_edit_not_found.patch_4.txt | 1.17 KB | mindless |
#5 | comment_edit_not_found.patch_3.txt | 1.13 KB | mindless |
#3 | comment_edit_not_found.patch_2.txt | 772 bytes | mindless |
Comments
Comment #1
Heine CreditAttribution: Heine commentedobjects without properties are no longer empty() in php5.
Comment #2
mindless CreditAttribution: mindless commentedguess you could use !isset. i don't really see when a comment object with no properties would be valid here..
Comment #3
mindless CreditAttribution: mindless commentedUpdated patch attached, against current HEAD rev for comment.module.
you can still get a series of php warnings with a comment/edit/{invalid#} URL.
Comment #4
Dries CreditAttribution: Dries commentedCan we write this as
if ($comment) { // do stuff } else { return drupal_not_found(); }
? That's more consistent with the rest of the Drupal code. Thanks.Comment #5
mindless CreditAttribution: mindless commentedattached.
Comment #6
mindless CreditAttribution: mindless commentedComment #7
mindless CreditAttribution: mindless commentedRerolled patch from current cvs.
Comment #8
mindless CreditAttribution: mindless commentedLet me know if anything more is needed on this one, thanks.
Comment #9
Freso CreditAttribution: Freso commentedI can reproduce the bug, but the current patch doesn't apply:
Re-rolled the patch which made it work as advertised. AFAICT, it's RTBC.
Comment #10
Gábor HojtsyNeither drupal_access_denied(), nor drupal_not_found() return anything. So why do we have return drupal_not_found() in the patch? It looks like being at the end of the function even.
Comment #11
Freso CreditAttribution: Freso commentedThis patch is identical to the last one, with the exception of one less
return
. I haven't tested that this new patch works or even applies, but it should work just as well as the other did. But as I haven't actually tested it, I'll just leave it as "needs review".Comment #12
catchNo longer applies.
Comment #13
dpearcefl CreditAttribution: dpearcefl commentedIs this still a problem in current D6?
Comment #14
dpearcefl CreditAttribution: dpearcefl commented