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.
The install script creates a hash that is supposed to be secret, but it's not quite super secret enough. Ideally it should use something like http://us.php.net/uniqid as part of the hash to make it harder (near impossible) to calculate.
Comment | File | Size | Author |
---|---|---|---|
#10 | 790586-fix-salts_1.patch | 1.18 KB | greggles |
#7 | 790586-fix-salts.patch | 1.16 KB | Dave Reid |
#6 | 790586-fix-salts.patch | 1.16 KB | Dave Reid |
#1 | 790586-better-install-1.patch | 1.34 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedsomething like this?
Comment #2
gregglesLooks great to me - thanks Peter!
Comment #4
gregglesFinally committed - thanks http://drupalcode.org/project/comment_notify.git/commit/7423926
Comment #5
gregglesapparently "uniqid(mt_rand, TRUE)" is not really the right thing to do.
#1261532: Error on new update install
@pwolanin, what was your intent here?
Comment #6
Dave ReidComment #7
Dave ReidBetter patch that uses COALESCE for non-pgsql since that's more standard that IFNULL and uppercases SQL functions for readability.
Comment #8
Dave ReidBumping to critical as this prevents the module from being installed properly.
Comment #9
Dave ReidAlso, it should be possible to do this if we nest CONCAT() statements using two arguments per call - then it's 100% DBTNG compatible and we don't have to special-case for PostgreSQL.
Comment #10
gregglesGreat. I feel like nesting concats is ugly enough that it offsets portability.
#7 didn't work perfectly b/c it leaves us with the same salt for notifications on the same node.
Attached version adds cid to provide even more uniqueness.
Comment #11
kingfisher64 CreditAttribution: kingfisher64 commentedI get the following error message when the module is enabled on drupal 7.8. I followed the link from http://drupal.org/node/1261532.
Notice: Use of undefined constant mt_rand - assumed 'mt_rand' in comment_notify_install()
Can someone tell me if the patch above is the fix to problem? If so could someone tell me how to apply this patch (i've never done a patch install before).
Thank you in advance for any help received
Comment #12
greggles@kingfisher64, yes, this is to fix that problem.
Thanks, Dave. This is now committed: http://drupalcode.org/project/comment_notify.git/commit/581f0ff