rails / rails

Ruby on Rails
https://rubyonrails.org
MIT License
56.13k stars 21.7k forks source link

ActionDispatch `recognize_path` trips over domain constraints #51117

Open svoop opened 9 months ago

svoop commented 9 months ago

Steps to reproduce

The following line of code may (under rare conditions) cause problems:

https://github.com/rails/rails/blob/44db6bbfcbe66ac5926f01f3b693a378552b752f/actionpack/lib/action_dispatch/routing/route_set.rb#L888

This mock request works fine for most Rails apps, however, if the app handles multiple domains and contains domain specific constraints, things go kaboom. Say you have a constraint like this one:

get '(*path)', to: "domains#redirect", constraints: ->(request) do
  request.host != 'myservice.com'
end

(Things such a DomainsController might do: Redirect "www.myservice.com" to "myservice.com" or redirect shortcuts such as "yourproject.myservice.com" to "myservice.com/yourproject".)

With such an entry in routes.rb, the mock request misbehaves because it is only fed with the path and therefore uses the hardcoded fallback server name "example.org". And since "example.org" != "myservice.com", the remainder of recognize_path works off the wrong route.

The solution is quite simple, something along the lines of:

# replace...
env = Rack::MockRequest.env_for(path, method: method)

# with...
env = Rack::MockRequest.env_for(url_helpers.root_url.delete(r{/$}) + path, method: method)

By passing the real URL instead of just the path, the mock request no longer uses the fallback but hits the router as a real request would.

I'll work around this for now and take a look at the corresponding tests to check whether I can solder together a PR.

System configuration

Rails version: 6.1.7.6 (but the code hasn't changed on main as of today)

Ruby version: 3.1.4

rails-bot[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-2-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

svoop commented 6 months ago

Nothing has changed, still a problem.

rails-bot[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-2-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

svoop commented 3 months ago

Keepalive, pls

rails-bot[bot] commented 1 week ago

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 8-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

svoop commented 1 week ago

Ping!