Make comment_* cookies pluggable

[pishmishy]

EU legislation dictates that some particular uses of cookies must first require consent from the user. The exact implementation of the EU legislation is dependent upon the individual member states and so there are a number of conflicting and confusing ideas over what types of cookies can be used with and without consent. See ​http://en.wikipedia.org/wiki/Directive_on_Privacy_and_Electronic_Communications for more details.

The functionality provided by the comment_ cookies set by WordPress is arguably not required for a WordPress site to function and in some parts of the EU consent may be required. Regardless of your view on the sensibilities of this we should allow site administrators to override the default behavior with a plugin if necessary.

(I am not proposing that the default behavior of WordPress be modified in any form in order to meet the requirements of any of this confusing legislation. See the forums for debates about this).

I'm attaching a patch that makes the calls to set_cookie from wp-comment-post.php pluggable. I've also got an example plugin, please let me know if posting it would be helpful.

[pishmishy]

Patch makes comment_ cookies pluggable

[dd32]

Whilst it adds a set-cookie to the response still, returning a negitive value on the comment_cookie_lifetime filter should also comply, as a browser would not store the cookie, and would delete any existing cookies.

However, we should avoid adding pluggable functions, and instead should alter the process to perform the cookie-adding routines in a function hooked to an action.

See Also: #16979

[pishmishy]

I was going down the action path before I wrote this patch but then changed my mind. I'm probably wrong :)

I think what put me off is that I couldn't remember how to make sure that my test plugin had the only function registered for that action. Is it just a case of calling remove_all_actions() and hoping that another plugin doesn't come along after it?

[dd32]

as an example, all I'd do is:

in default-filters:
add_action('commented_on_post', 'add_commenter_cookies'); (need to check that action)

in a plugin on an appropriate hook (probably init?)
remove_action('commented_on_post', 'add_commenter_cookies');

and put the add_commenter_cookies function into the comments include file..

[pishmishy]

Thanks. I'll work on a better patch.

[pishmishy]

Patch using actions

[pishmishy]

Something like the attached? I can't quite get it working though - not sure what I'm doing wrong.

[SergeyBiryukov]

Revised pishmishy's previous patch. Added missing global variable and adjusted to comply with our coding standards.

[pishmishy]

Thank you - I knew I was missing something.

I've been able to write a plugin to work with this patch and it appears to work well. I'm asking some colleagues to test the patch and my plugin to see if it allows them to control the use of cookies in the ways they'd like to, whilst still retaining the default behavior.

[westi]

I don't think using the comment_post action is correct here we should instead have an action which is specific to the wp-comment-posts.php script for setting the cookies and then plugins can just do a simple remove action call to filter out the cookie setting.

[SergeyBiryukov]

Added a separate action in 17976.2.patch​.

[WhiteJV]

If we were to wish to not set cookies would the following work from within mu-plugins?

remove_action( 'set_comment_cookies', 'set_comment_cookies' );

If so, this will do well to allow us to not set new cookies, however, due to the long TTL of the cookie, there is a potential to expose personally identifiable information of users who have commented in the past but have never flushed their cookies. Could this patch be expanded to also include a pluggable function for reading comment cookies?

[SergeyBiryukov]

Replying to WhiteJV:

If we were to wish to not set cookies would the following work from within mu-plugins?

Yes, that's the idea. Refreshed the patch for current trunk.

[westi]

Thanks for the updated patch

I think it would be nicer to pass the 'user' object to the action and therefore the function rather than relying on the name of a global so that the cookie setting is independent of any globals.

[SergeyBiryukov]

Replying to westi:

I think it would be nicer to pass the 'user' object to the action and therefore the function rather than relying on the name of a global so that the cookie setting is independent of any globals.

Thanks for the feedback, done in 17976.4.patch​.

Would it make sense to pass the whole comment object as well, rather than call get_comment() for the second time? If no, I'll replace it with $comment_id again.

[SergeyBiryukov]

Replying to WhiteJV:

If so, this will do well to allow us to not set new cookies, however, due to the long TTL of the cookie, there is a potential to expose personally identifiable information of users who have commented in the past but have never flushed their cookies. Could this patch be expanded to also include a pluggable function for reading comment cookies?

It's possible to hook into wp_get_current_commenter() function to ignore existing cookies:

function ignore_existing_comment_cookies() {
	return array( 'comment_author' => '', 'comment_author_email' => '', 'comment_author_url' => '' );
}
add_filter( 'wp_get_current_commenter', 'ignore_existing_comment_cookies' );

[jane]

@westi: It seems awfully late to be trying to squeeze this in, as we're already in 2nd beta, on the march toward RC. By our own definitions of each phase, this should be bumped to 3.4-early.

[nacin]

17976.4.patch looks good. Two suggestions: Bail early if $user->ID, and prefix function (though not action) with wp_.

[SergeyBiryukov]

Thanks, refreshed the patch.

[westi]

In [19622]:

Make it possible for commenter cookies to be disabled if someone wants to by setting them on an action instead of always. Fixes #17976 props SergeyBiryukov and pishmishy .