Skip to content

Conversation

@People-Sea
Copy link
Member

@People-Sea People-Sea commented Jan 29, 2026

Description

Fixes: #19117

Compare URL host with request()->getHost() instead of using config('app.url') prefix matching.

Visual changes

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@github-project-automation github-project-automation bot moved this to Todo in Roadmap Jan 29, 2026
@People-Sea People-Sea changed the title fix: cross-domain URL detection in multi-domain apps [4.x] Fix cross-domain URL detection in multi-domain apps Jan 29, 2026
@People-Sea People-Sea marked this pull request as ready for review January 29, 2026 10:16
@danharrin danharrin added the bug Something isn't working label Jan 29, 2026
@danharrin danharrin added this to the v4 milestone Jan 29, 2026
Copy link
Member

@danharrin danharrin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind elaborating on the difference between request()->root() and request()->getHost(), and if the issue in #19024 will return?

Maybe @eslam-reda-div wants to have a look and see if this still fixes their problem

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Roadmap Jan 29, 2026
@eslam-reda-div
Copy link
Contributor

Would you mind elaborating on the difference between request()->root() and request()->getHost(), and if the issue in #19024 will return?

Maybe @eslam-reda-div wants to have a look and see if this still fixes their problem

I plan to test this change across multiple runtime environments (Octane + FrankenPHP + Docker + Nginx) to make sure everything works properly.

From my initial review, this solution is better than the previous approach, as it handles multi-tenant setups, proxies, and subdomains more accurately and reliably.

I’ll share the test results once done. If any issues come up during testing, I’ll report them immediately or provide adjustments as needed.

The only important requirement is ensuring that trusted proxies are configured correctly so that request()->getHost() returns the correct host behind reverse proxies.

@People-Sea
Copy link
Member Author

@danharrin
request()->root() returns full URL (https://example.com), request()->getHost() returns just the host (example.com). This fix compares hosts directly via parse_url() instead of prefix matching.
The scenario in #19024 is actually a configuration issue — reverse proxies need to forward the correct Host header, or TrustProxies needs to be configured. Most standard setups already handle this correctly.

@danharrin danharrin merged commit 8d289ac into filamentphp:4.x Jan 29, 2026
24 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Roadmap Jan 29, 2026
@eslam-reda-div
Copy link
Contributor

@danharrin
request()->root() returns full URL (https://example.com), request()->getHost() returns just the host (example.com). This fix compares hosts directly via parse_url() instead of prefix matching.
The scenario in #19024 is actually a configuration issue — reverse proxies need to forward the correct Host header, or TrustProxies needs to be configured. Most standard setups already handle this correctly.

I had already configured everything related to trusted proxies, but the issue would still fail in some runtime environments when multiple instances were behind a load balancer. The workaround I had implemented previously compared URLs to APP_URL, because in certain internal requests, Laravel Octane would send a different internal host than the main domain.

I’ll re-test this new fix to confirm that it correctly resolves the host internally within the runtime and that it won’t conflict with Octane. This should ensure that the host is consistently identified, even behind load balancers or in multi-instance setups.

@People-Sea People-Sea deleted the fix/issues/19117 branch January 29, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug] v4.6.0 breaks cross-domain URL actions in multi-domain applications

3 participants