Skip to content

Conversation

@chuoke
Copy link
Contributor

@chuoke chuoke commented Sep 23, 2024

When using REMOTE_ADDR set the trust proxy in Swoole, like this:

 $middleware->trustProxies( at: [ 'REMOTE_ADDR', ], headers: Request::HEADER_X_FORWARDED_FOR );

Is nothing set to request->trustedProxies because $_SERVER['REMOTE_ADDR'] does not exist in Swoole. But Laravel internally converts the server information in Swoole into the request->server attribute, which includes REMOTE_ADDR, and then request()->server->get('REMOTE_ADDR') can get the corresponding value normally.

This leads to some logical ambiguity and increases the cognitive burden and learning cost.

So we need to unify the logic to avoid inconsistency. I can't change this in the Request class because of the method attribute restrictions, so the solution I've come up with so far is this, it works for me, and I hope there's another better way to solve it.

@crynobone crynobone changed the title fix: trust proxy REMOTE_ADDR not working in Swoole [11.x] Fixes trust proxy REMOTE_ADDR not working in Swoole Sep 24, 2024
@crynobone crynobone changed the title [11.x] Fixes trust proxy REMOTE_ADDR not working in Swoole [11.x] Fixes trust proxy REMOTE_ADDR not working in Swoole Sep 24, 2024
@taylorotwell
Copy link
Member

Can you add a test then mark as ready for review?

@taylorotwell taylorotwell marked this pull request as draft September 25, 2024 15:24
@chuoke chuoke marked this pull request as ready for review September 26, 2024 06:47
@chuoke
Copy link
Contributor Author

chuoke commented Sep 27, 2024

hi @taylorotwell I added one test case. the checks were unsuccessful, but not caused by this change, it seems some other network is wrong. Can you recheck this?

@taylorotwell taylorotwell merged commit 7aabb89 into laravel:11.x Sep 27, 2024
30 checks passed
@chuoke chuoke deleted the fix-trust-proxy-remote_addr-swoole branch September 29, 2024 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants