Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  José Valim February 10th, 2010 @ 06:48 AM

  • Milestone cleared.
  • Assigned user set to “Yehuda Katz (wycats)”
• 

  Dan Pickett May 9th, 2010 @ 06:22 PM

  • Tag set to “bugmash”
• 

  Rohit Arondekar May 15th, 2010 @ 11:37 AM

  Confirmed on Rails 3 beta 3 on ruby 1.9.2dev (2010-05-08 trunk 27665)

  Took the following steps (or skip to the attached rails 3 beta 3 app)

  rails forgery_3913
  rails generate scaffold Post title:string content:text
  rails generate controller Imposter new

  Added protect_from_forgery :except => :create to posts_controller

  Edited app/views/imposter/new.html.erb to have => http://pastie.org/961450

  rails server

  Go to http://localhost:3000/imposter/new and try submitting the form. It throws a ActionController::InvalidAuthenticityToken in PostsController#create

  P.S Find attached a rails 3 b3 app. Do rails server and try submitting from the above url. Should not work.

  • forgery_3913.zip 102.6 KB

• You flagged this item as spam.
  

  Anil Wadghule May 15th, 2010 @ 01:12 PM

  Verified this issue on Rails head. But looks like it's an issue with before_filter. Attached is an rails app showing issue with before filter.

  in ApplicationController,

  I have added

  before_filter :set_message

  def set_message
  @message = "I am set" end

  and in UsersController,

  before_filter :set_message, :except => :index

  http://localhost:3000/users/ should not print "I am set". But it is printing.

• You flagged this item as spam.
  

  Anil Wadghule May 15th, 2010 @ 01:13 PM

  Attached app here

  • before_filter_3913.zip 101.6 KB
• 

  Ryan Bigg May 17th, 2010 @ 10:36 AM

  I can confirm this issue exists too.

• 

  Ryan Bigg May 17th, 2010 @ 10:37 AM

  • State changed from “new” to “open”
• 

  Rohit Arondekar May 18th, 2010 @ 06:31 AM

  I have a failing test for this issue. I've attached it as a patch. I've tested it on ruby 1.9.2dev (2010-05-08 trunk 27665) [x86_64-linux]

  P.S This is my first addition of a test so if I've not followed any conventions or need to make changes feel free to let me know and I'll do them.

  • 3913_failing_test.diff 1.8 KB
• 

  Rohit Arondekar May 18th, 2010 @ 06:33 AM

  I forgot to mention that it's not for the original issue, but rather for the issue Anil reported regarding the before_filter. Since it looks like this is cause the original issue.

• You flagged this item as spam.
  

  Anil Wadghule May 18th, 2010 @ 12:21 PM

  Rohit, your test patch looks good. But you can write it in filters_test.rb instead of callbacks_test.rb.

• 

  Rohit Arondekar May 18th, 2010 @ 02:19 PM

  I have a few questions.

  (1) Given that ApplicationController has a before_filter :foo and BooController < ApplicationController has a before_filter :foo, how does Rails know that the second before_filter shouldn't be executed?

  (2) If I tell you that the before_filter of ApplicationController is being run before the before_filter of a specific controller has the chance to specify :except, would I be talking sense or am I talking crazy?

  I have reason to believe that (2) is happening. But I can't explain why exactly because I've used rudimentary echoing of vars in callbacks.rb to arrive at that conclusion. If what I've said is of any help to arrive at a solution, then great! But I could be talking total nonsense and if I am please ignore me :P

• 

  Rohit Arondekar May 20th, 2010 @ 01:39 AM

  I have a little more vague information.

  I think the "appending" that's happening is to the filter chain. Or is there such a thing as a callback chain too?

  I think before the specific before_filter gets a chance to provide the :except option, the before_filter of the ApplicationController is being executed. But it doesn't execute the before_filter twice, so there is a check for this?

  Can somebody point to the code where the filter chain is processed/executed?

• 

  Rohit Arondekar May 20th, 2010 @ 10:00 AM

  Some more info:

  I think I'm certain now that filters are defined over callbacks. See api comment for set_callback in activesupport/lib/active_support/callbacks.rb

  
  

    # When creating or skipping callbacks, you can specify conditions that
    # are always the same for a given key. For instance, in ActionPack,
    # we convert :only and :except conditions into per-key conditions.
    #
    #   before_filter :authenticate, :except => "index"
    # becomes
    #   dispatch_callback :before, :authenticate, :per_key => {:unless => proc {|c| c.action_name == "index"}}
  

  Also I think this is the method that is somehow not removing the before_filter of the ApplicationController even though there is a before_filter with :except option in a specific controller.

  
  

    def skip_callback(name, *filter_list, &block)
      __update_callbacks(name, filter_list, block) do |chain, type, filters, options|
        filters.each do |filter|
          filter = chain.find {|c| c.matches?(type, filter) }
  
          if filter && options.any?
            new_filter = filter.clone(chain, self)
            chain.insert(chain.index(filter), new_filter)
            new_filter.recompile!(options, options[:per_key] || {})
          end
  
          chain.delete(filter)
          send("_removed_#{name}_callbacks") << filter
        end
      end
    end
  

  I don't know how long it will take to me to understand the whole flow completely, so I thought I would just put it in here for anyone else who might know how to fix it.

• 

  Rohit Arondekar June 9th, 2010 @ 08:32 AM

  I don't think there is any code that will remove the before_filter set by ApplicationController even though it's subclasses have one defined, with exceptions. Can anybody point me to the code that does this?

• 

  José Valim June 9th, 2010 @ 08:37 AM

  • Milestone cleared.
  • Assigned user changed from “Yehuda Katz (wycats)” to “José Valim”

  The code does not remove the previous before filter, but should update it. This is a serious bug, I should work on it with Yehuda.

• You flagged this item as spam.
  

  Bruno Michel June 11th, 2010 @ 05:05 PM

  FWIW, I'd always used:

  skip_before_filter :verify_authenticity_token, :only => [:method]
  

  for skiping this filter. I'm not convinced that declaring the same filter two time should merge their :only|:except, skip_before_filter is here for that, no?

• 

  Rohit Arondekar June 22nd, 2010 @ 06:44 AM

  Was this fixed in master? I can't reproduce this anymore.

• 

  Repository June 22nd, 2010 @ 06:58 AM

  • State changed from “open” to “resolved”

  (from [02399a1184d0f87b14af461fab731120d92c5ad8]) Ensure overwritten callbacks conditions in controllers work [#4761 state:resolved] [#3913 state:resolved] http://github.com/rails/rails/commit/02399a1184d0f87b14af461fab7311...

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”
  • Importance changed from “” to “High”

  [bulk edit]