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

Sending problematic method names (open, format) to decorated rails objects sends to Kernel (conflicts with ransack) #887

Open
klyonrad opened this issue Aug 18, 2020 · 0 comments

Comments

@klyonrad
Copy link

klyonrad commented Aug 18, 2020

I ran into this very surprising issue last week. I added the attribute open to a model and then to the edit form of that model.

Upon opening that edit form I saw an ArgumentError and the stack trace indicates that somehow the method open from open-uri got called.

The stack trace also included a monkey patch from the ransack gem so it was a bit of bad luck that I ran into this issue. They replaced the public_send call with send which is why this bug does not appear in a normal rails installation with draper.

So I tried to create a bug report for them and played detective for way too long 🕵️. Then I saw it! We also use the decorates_assigned method in the culprit controller and the object that gets passed to the form is actually a Decorator object and not really the model.

Probably I will open an issue in the ransack project as well but I believe that this handling of the send behaviour is undesired. Of course I can work around the bug for this particular case but there might be other metaprogramming somewhere that uses send and not public_send on decorated objects - especially in the context of views.

Appendix

Problematic Draper behaviour

class SomeObject
  def open
    'false'
  end  
end
class SomeObjectDecorator
  delegate_all
end
SomeObject.new.send('open')
=> "false"
SomeObjectDecorator.new(SomeObject.new).send('open')
ArgumentError: wrong number of arguments (given 0, expected 1+)
from .../ruby-2.6.6/lib/ruby/2.6.0/open-uri.rb:29:in `open'

rails integration test

Personally, I tested this with draper 3.x and 4.x.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", '~> 5.2'
  # Activate the gem you are reporting the issue against.
  gem "sqlite3"

  gem 'hamlit'
  gem 'discard' # loading/requiring ransack fails without this. No idea why.
  gem 'draper'
  gem 'ransack'
end

require "active_record"
require "rack/test"
require "action_controller/railtie"

class TestApp < Rails::Application
  config.root = __dir__
  # config.hosts << "example.org"
  config.session_store :cookie_store, key: "cookie_store_key"
  secrets.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  routes.draw do
    resources :comments
  end
end

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :comments, force: true do |t|
    t.string :open, default: 'false'
    t.string :author_name
  end
end

class Comment < ActiveRecord::Base
  include Draper::Decoratable
end

class CommentDecorator < Draper::Decorator
  delegate_all
end

class CommentsController < ActionController::Base
  include Rails.application.routes.url_helpers
  include Draper::DecoratesAssigned # grrr not working
  # decorates_assigned :comment

  def index
    render plain: "Home"
  end

  def edit
    @comment = Comment.find(params[:id]).decorate

    template = <<ERB
<%= form_for(@comment) do |f| %>
  <%= f.text_field :open %>
<% end %>
ERB
    render inline: template, layout: false
  end
end

require "minitest/autorun"

class BugTest < Minitest::Test
  include Rack::Test::Methods

  def test_smoke
    get "/comments"
    assert last_response.ok?
  end

  def test_returns_success
    comment = Comment.create!(open: 'true')
    get "/comments/#{comment.id}/edit"
    assert last_response.ok?
  end

  private

  def app
    Rails.application
  end
end

Stacktrace

test leads to

F, [2020-08-18T17:39:40.429260 #14860] FATAL -- : ActionView::Template::Error (wrong number of arguments (given 0, expected 1..3)):
F, [2020-08-18T17:39:40.429340 #14860] FATAL -- :     1: <%= form_for(@comment) do |f| %>
    2:   <%= f.text_field :open %>
    3: <% end %>
F, [2020-08-18T17:39:40.429357 #14860] FATAL -- :   
F, [2020-08-18T17:39:40.429388 #14860] FATAL -- : ransack (2.3.2) lib/ransack/helpers/form_builder.rb:14:in `initialize'
ransack (2.3.2) lib/ransack/helpers/form_builder.rb:14:in `open'
ransack (2.3.2) lib/ransack/helpers/form_builder.rb:14:in `value'
actionview (5.2.4.3) lib/action_view/helpers/tags/base.rb:53:in `value_before_type_cast'
actionview (5.2.4.3) lib/action_view/helpers/tags/text_field.rb:15:in `block in render'
actionview (5.2.4.3) lib/action_view/helpers/tags/text_field.rb:15:in `fetch'
actionview (5.2.4.3) lib/action_view/helpers/tags/text_field.rb:15:in `render'
actionview (5.2.4.3) lib/action_view/helpers/form_helper.rb:1136:in `text_field'
actionview (5.2.4.3) lib/action_view/helpers/form_helper.rb:1680:in `text_field'
@klyonrad klyonrad changed the title Sending problematic method names (open, format) to decorated rails objects send to Kernel (conflicts with ransack) Sending problematic method names (open, format) to decorated rails objects sends to Kernel (conflicts with ransack) Aug 18, 2020
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

2 participants