xwp / stream

🗄️ Stream plugin for WordPress
https://wordpress.org/plugins/stream/
GNU General Public License v2.0
405 stars 119 forks source link

Improve client IP resolution for Stream logs #1459

Closed kasparsd closed 8 months ago

kasparsd commented 9 months ago

Fixes #1456.

Checklist

Release Changelog

kasparsd commented 8 months ago

@schlessera I feel like attempting the fallback behaviour might not be in our best interest here because it would actually be different from the previous behaviour. Let's make it a breaking change instead (does this warrant a 4.0.0 release?) and encourage everyone to use the newly introduced filter for adjusting the client IP address on environments where either REMOTE_ADDR is reporting wrong.

calvinalkan commented 8 months ago

@schlessera I feel like attempting the fallback behaviour might not be in our best interest here because it would actually be different from the previous behaviour. Let's make it a breaking change instead (does this warrant a 4.0.0 release?) and encourage everyone to use the newly introduced filter for adjusting the client IP address on environments where either REMOTE_ADDR is reporting wrong.

My 2C.

  1. The fallback behaviour is insecure. People wil shoot themselves in the foot.
  2. If PHP can't trust it's server to set REMOTE_ADDR correctly, or at all, all hope is lost. This should not be a consideration.
  3. Yes, breaking change, 4.0.0