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

Mounted Engine named_routes Regression in v4.2.3 #20920

Closed
merhard opened this issue Jul 17, 2015 · 2 comments
Closed

Mounted Engine named_routes Regression in v4.2.3 #20920

merhard opened this issue Jul 17, 2015 · 2 comments

Comments

@merhard
Copy link
Contributor

merhard commented Jul 17, 2015

When running a Rails v4.2.2 app under a relative url with an isolated mounted engine, the engine named route helpers generate the correct path. This is no longer the case in Rails v4.2.3 or 4-2-stable.

I believe the change in behavior stems from this commit: 0703453.

Per 5b3bb61, mounted engines use options[:original_script_name] for the prefix generation instead of options[:script_name]. Defaulting an absent options[:script_name] with relative_url_root causes the code to include the relative url twice in the generated path (e.g. "/sub_uri/sub_uri/engine/route" instead of "/sub_uri/engine/route").

I have created a test app with a failing test to demonstrate this issue: https://github.com/merhard/rails-regression-testapp

Pull request for fix: #20958

@shawnstephens
Copy link

I'm experiencing a similar issue with a mounted engine. Using Passenger 4.0.59 I get paths from the url_helpers of the engine with the duplicated "/sub_uri/sub_uri/engine/route" path.

merhard pushed a commit to merhard/rails that referenced this issue Jul 20, 2015
When generating the url for a mounted engine through its proxy (e.g. foo.bar_path), the relative_url_root should not be used when generating the proxy’s url prefix.

(fixes rails#20920)
merhard pushed a commit to merhard/rails that referenced this issue Sep 23, 2015
When generating the url for a mounted engine through its proxy, the path should be the sum of three parts:

1. Any `SCRIPT_NAME` request header or the value of `relative_url_root`.
2. A prefix (the engine's mounted path).
3. The path of the named route inside the engine.

Since commit rails@0703453, this has been broken. Instead, the path is the sum of four parts:

1. Any `SCRIPT_NAME` request header or the value of `relative_url_root`.
2. The value of `relative_url_root`.
3. A prefix (the engine's mounted path).
4. The path of the named route inside the engine.

This commit fixes the regression by providing a benign truthy value for `options[:script_name]` (an empty string) when the prefix is being generated, avoiding the duplicate `relative_url_root` value in the final result.

This resolves rails#20920, resolves rails#21459, and closes rails#20958
@merhard
Copy link
Contributor Author

merhard commented Sep 24, 2015

Fyi, I added a new pull request with a different solution to this issue. #21743 Hopefully this issue can be resolved soon.

merhard pushed a commit to merhard/rails that referenced this issue Sep 28, 2015
When generating the url for a mounted engine through its proxy, the path should be the sum of three parts:

1. Any `SCRIPT_NAME` request header or the value of `relative_url_root`.
2. A prefix (the engine's mounted path).
3. The path of the named route inside the engine.

Since commit rails@44ff031, this has been broken. Instead, the path is the sum of four parts:

1. Any `SCRIPT_NAME` request header or the value of `relative_url_root`.
2. The value of `relative_url_root`.
3. A prefix (the engine's mounted path).
4. The path of the named route inside the engine.

This commit fixes the regression by providing a benign truthy value for `options[:script_name]` (an empty string) when the prefix is being generated, avoiding the duplicate `relative_url_root` value in the final result.

This resolves rails#20920, resolves rails#21459, and closes rails#20958
pixeltrix pushed a commit that referenced this issue Oct 8, 2015
When generating the url for a mounted engine through its proxy, the path should be the sum of three parts:

1. Any `SCRIPT_NAME` request header or the value of `ActionDispatch::Routing::RouteSet#relative_url_root`.
2. A prefix (the engine's mounted path).
3. The path of the named route inside the engine.

Since commit 44ff031, this has been broken. Step 2 has been changed to:

2. A prefix (the value of `ActionDispatch::Routing::RouteSet#relative_url_root` + the engine's mounted path).

The value of `ActionDispatch::Routing::RouteSet#relative_url_root` is taken into account in step 1 of the route generation and should be ignored when generating the mounted engine's prefix in step 2.

This commit fixes the regression by having `ActionDispatch::Routing::RouteSet#url_for` check `options[:relative_url_root]` before falling back to `ActionDispatch::Routing::RouteSet#relative_url_root`. The prefix generating code then sets `options[:relative_url_root]` to an empty string. This empty string is used instead of `ActionDispatch::Routing::RouteSet#relative_url_root` and avoids the duplicate `relative_url_root` value in the final result.

This resolves #20920 and resolves #21459
(cherry picked from commit bcfbd8b)
ijdickinson added a commit to epimorphics/qonsole-rails that referenced this issue Nov 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants