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.

Files: 
CommentFileSizeAuthor
#10 790586-fix-salts_1.patch1.18 KBgreggles
#7 790586-fix-salts.patch1.16 KBDave Reid
#6 790586-fix-salts.patch1.16 KBDave Reid
#1 790586-better-install-1.patch1.34 KBpwolanin
FAILED: [[SimpleTest]]: [MySQL] 3 pass(es), 5 fail(s), and 6 exception(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.34 KB
FAILED: [[SimpleTest]]: [MySQL] 3 pass(es), 5 fail(s), and 6 exception(es).
[ View ]

something like this?

Looks great to me - thanks Peter!

Status:Needs review» Needs work

The last submitted patch, 790586-better-install-1.patch, failed testing.

Status:Needs work» Fixed

Status:Fixed» Needs work

apparently "uniqid(mt_rand, TRUE)" is not really the right thing to do.

#1261532: Error on new update install

@pwolanin, what was your intent here?

Status:Needs work» Needs review
StatusFileSize
new1.16 KB

StatusFileSize
new1.16 KB

Better patch that uses COALESCE for non-pgsql since that's more standard that IFNULL and uppercases SQL functions for readability.

Priority:Normal» Critical

Bumping to critical as this prevents the module from being installed properly.

Also, 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.

StatusFileSize
new1.18 KB

Great. 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.

I 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

Status:Needs review» Fixed

@kingfisher64, yes, this is to fix that problem.

Thanks, Dave. This is now committed: http://drupalcode.org/project/comment_notify.git/commit/581f0ff

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.