rails
rails copied to clipboard
`ActionDispatch::Routing::RoutesProxy#merge_script_names` throws exceptions given certain arguments
Steps to reproduce
- Mount an engine in the routes file inside of a namespace (or sub path)
- Attempt to call a top-level route from the engine's route helper
- A runtime exception occurs -- NoMethodError: undefined method `join' for nil:NilClass
Test Case
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails", github: "rails/rails", branch: "main"
end
require "active_support"
require "active_support/core_ext/object/blank"
require "minitest/autorun"
require "rails"
class BugTest < Minitest::Test
def test_stuff
proxy = ActionDispatch::Routing::RoutesProxy.new(nil, nil, nil)
assert_equal "/foo/bar", proxy.send(:merge_script_names, "", "/foo/bar") #=> NoMethodError: undefined method `join' for nil:NilClass
end
end
Expected behavior
The engine's route helper (which is an instance of ActionDispatch::Routing::RoutesProxy
) should return the requested url or path without throwing an exception.
Actual behavior
https://github.com/rails/rails/blob/18707ab17fa492eb25ad2e8f9818a320dc20b823/actionpack/lib/action_dispatch/routing/routes_proxy.rb#L58-L66
-
#merge_script_names
is invoked with two arguments,""
and"/foo/bar"
-
resolved_parts
evaluates to2
-
previous_parts
evaluates to0
-
context_parts
evaluates to-1
(!!!) -
previous_script_name.split("/")
evaluates to an empty array -
Array#slice(0, -1)
is called, which evaluates tonil
(but an array was expected) -
#join
is invoked on an instance ofNilClass
, because#slice
did not return an array due to the negative value of the second argument
Suggested Remediation
context_parts
should probably be guarded to ensure a negative value isn't generated -- maybe something like:
context_parts = [0, previous_parts - resolved_parts + 1].max
System configuration
Rails version: 7.0.3
Ruby version: ruby 3.0.4p208 (2022-04-12 revision 3fa771dded) [arm64-darwin21]
Hi @kenaniah , thanks for opening this issue!
It looks like your reproduction script is using some private Rails APIs like ActionDispatch::Routing::RoutesProxy
. I'm not that familiar with the internals of ActionDispatch so it would be nice to get a reproduction script using public APIs from a user's standpoint.
I started a reproduction script here, but I wasn't sure what you meant in step 2 of the steps to reproduce
- Attempt to call a top-level route from the engine's route helper
Are you able to expand this script to show the issue?
Reproduction script
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails", github: "rails/rails", branch: "main"
end
require "action_controller/railtie"
module Blorgh
class Engine < ::Rails::Engine
# Patch to make repro script work
def self.find_root(from)
Pathname.new(from).expand_path
end
isolate_namespace Blorgh
routes.draw do
get "/" => "posts#index"
end
end
class PostsController < ActionController::Base
def index
render plain: "Hello, world!"
end
end
end
class TestApp < Rails::Application
config.root = __dir__
config.hosts << "example.org"
secrets.secret_key_base = "secret_key_base"
config.logger = Logger.new($stdout)
Rails.logger = config.logger
routes.draw do
namespace :example do
mount Blorgh::Engine => "/blorgh"
end
get "/" => "test#index"
end
end
class TestController < ActionController::Base
include Rails.application.routes.url_helpers
def index
render plain: "Home"
end
end
require "minitest/autorun"
require "rack/test"
class BugTest < Minitest::Test
include Rack::Test::Methods
def test_returns_success
get "/example/blorgh"
assert last_response.ok?
end
private
def app
Rails.application
end
end
@luanzeba I looked into creating a test case using just the public APIs, but frankly that's kind of irrelevant here. This report comes from a backtrace from one of my Rails 7 applications that happens to have a wild and wonderful routing tree (using multiple engines for subroutes that are dynamically generated via meta-programming).
Reducing the test case in the public API would depend on understanding how script names are inferred, which is not something I was able to figure out in the time I had allocated to this bug report. Regardless, my initial report should be sufficient for demonstrating the issue.
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-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.