Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remote Addr are not handled the same since akrabat/ip-address-middleware >= 2.5 #2351

Open
ymage opened this issue Feb 2, 2025 · 1 comment · May be fixed by #2352
Open

Remote Addr are not handled the same since akrabat/ip-address-middleware >= 2.5 #2351

ymage opened this issue Feb 2, 2025 · 1 comment · May be fixed by #2352
Labels

Comments

@ymage
Copy link

ymage commented Feb 2, 2025

Shlink version

version >=4.4.0

PHP version

8.3

How do you serve Shlink

Self-hosted RoadRunner

Database engine

MySQL

Database version

11.6

Current behavior

Shlink run in docker container which is reverse-proxified with traefik/nginx.
X-Real-IP and X-Forwarded-For are both correctly set with the client address and REMOTE_ADDR is the proxy address.
When the visit is tracked, only the REMOTE_ADDR is registered, which is a 172.16.0.0/12 non routed IP address since akrabat/ip-address-middleware >= 2.5 (akrabat/ip-address-middleware#51)

Expected behavior

Shlink should allow to set trusted_proxies to akrabat/ip-address-middleware in order to get it to ignore them and retrieve the X-Real-IP header IP address

Minimum steps to reproduce

Run a shlink container behind a reverse-proxy with a private class IP address which can't override REMOTE_ADDR.

@acelaya
Copy link
Member

acelaya commented Feb 3, 2025

So I guess your point is that, after those changes in akrabat/ip-address-middleware, the X-Real_IP and X-Forwarded-For headers are being ignored in some cases?

Can you provide a failing test case that proves it? This is probably the best test to extend

$ipAddressConfig = require __DIR__ . '/../../../../config/autoload/ip-address.global.php';

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants