anti-csrf icon indicating copy to clipboard operation
anti-csrf copied to clipboard

$post and $server are passed by reference in constructor for no reason

Open pmlt opened this issue 7 years ago • 1 comments

Hello,

First of all, thank you for the quality library. I rarely have such a high confidence level for a PHP library after perusing its code.

This is probably a nitpick, but I notice that in the constructor of the AntiCSRF class, both $post and $server are passed by reference. However, there seems to be no code which writes to these variables. This is misleading, because usually passing by reference in PHP is a way to inform users of a class that the value will get mutated.

I think this is important because in some execution environments (for example: automated tests), $_SERVER will not contain the expected information such as REMOTE_ADDR or REQUEST_URI. As such, I need to know that it's okay to simply pass a made-up array that simulates the structure of $_SERVER without impacting the functionality of the library. But the pass-by-reference semantics gives me the opposite impression.

I believe only $session should be passed by reference in the constructor of AntiCSRF.

pmlt avatar Jan 24 '18 22:01 pmlt

This was introduced in https://github.com/paragonie/anti-csrf/pull/9 with the goal of PSR-7 support.

paragonie-scott avatar Feb 06 '18 17:02 paragonie-scott