rails: Defining a `config` action on a controller causes a `SystemStackError`

Steps to reproduce

Generate a new Rails app (example Rails 5.2 app, example Rails 6.1 app)
```
rails new config-demo --minimal
```

Create a new controller with two actions, with at least one named config, which render something simple

class MyConfigController < ApplicationController
  def notconfig
    render text: :notconfig
  end

  def config
    render text: :config
  end
end

Create routes for the new controller and actions

get "/config, to: "my_config#config"
get "/notconfig, to: "my_config#notconfig"

Create a new ActionDispatch test file that calls the new routes

require "test_helper"

class MyConfigControllerTest < ActionDispatch::IntegrationTest
  def test_notconfig
    get "/notconfig"
    assert_equal 200, status
  end

  def test_config
    get "/config"
    assert_equal 200, status
  end
end

Run the test
```
bin/rails test
```

Expected behavior

I would expect the routes to work as intended, or for Rails to generate an error about overriding the config method as an action.

Actual behavior

Both new routes result in a SystemStackError: stack level too deep pointing to the config action definition. This is because ActiveSupport defines ActiveSupport::Configurable#config which is included into ActionController::Base by default, and the new config action overrides it. (We can confirm this by adding a byebug breakpoint in the config action and calling method(:config).super_method from the debugger console.) Our actions both call render which eventually will call logger which is a configurable attribute which calls config which then calls the new config action which calls render and so on. I initially spotted this when a team mate tried to create a config endpoint using https://github.com/github/chatops-controller, which essentially works the same way by defining a new action on the controller.

System configuration

Rails version: 5.2.4.4, 6.1.1, 6.2.0.alpha

Ruby version: 2.7.1, 2.6.6

Updated to show that this affects other actions in the same controller and does not rely on calling the config action directly

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 23 (20 by maintainers)

Commits related to this issue

Add note about action names versus ActionController reserved methods In both Rails 5.2 and Rails 6.1, defining a controller action method named `config` will result in a `SystemStackError: stack leve... — committed to chrisbloom7/rails by chrisbloom7 3 years ago

Most upvoted comments

https://github.com/rails/rails/blob/main/actionpack/lib/abstract_controller/base.rb#L156-L297 is a good start, but there are many others in ActionController::Metal and ActionController::Base.

rafaelfranca on Feb 5, 2021

@chrisbloom7 I need a bit more time to review this, but unfortunately yes a PR will make things easier once I can see the code 🙇 Thanks for your effort!

zzak on Jun 14, 2021

I wanted to get a sense of how many methods would result in something like what redefining config as an action method does, mainly to see if the list would be small enough to include in full in the new callout section. To help figure that out I generated a new rails app that dynamically creates controllers, actions, and routes for each instance method on ActionController::Base, with an integration test suite with a custom reporter that would exercise each route and dump failures to a CSV file. In Rails 6.1.3 there are 17 methods (including config) that would result in SystemStackError: stack level too deep if redefined in a controller, 9 that would result in a ArgumentError if redefined, and 3 that would result in a AbstractController::DoubleRenderError. Most of these are obvious, like yeah don’t redefine class or render or action_name. TBH the only one that jumps out at me as surprising is the one I originally encountered: config. So it’s probably not worth mentioning specific method names, but I still feel it’s worth calling out that there is a potential for accidentally redefining one of these methods and suddenly getting hit with ambiguous SystemStackError in all actions in the controller. Working on something along the lines of the text below. Will refine it and get a PR up in the morning

While it’s possible to define action methods of any name, some method names are reserved by Action Controller. Because of this, limiting your controllers to the using only the default RESTful [Resource Routing][] actions do not need to worry about this, but if your application defines custom routes then you should be aware of the potential for collision with a reserved method. If you find that your application would benefit from using a reserved method as an action name, one workaround is to define your action with a non-reserved method name and then use a custom route to map the otherwise reserved method name to your non-reserved action method.

chrisbloom7 on Jun 14, 2021

@zzak thanks for the reminder. I haven’t yet, but I can get to it this week.

chrisbloom7 on Jun 10, 2021