ActionController::TestCase::Behavior#process can modify its parameter #2624

Closed
dmajda opened this Issue Aug 22, 2011 · 4 comments

Comments

Projects
None yet
4 participants
Contributor

dmajda commented Aug 22, 2011

The ActionController::TestCase::Behavior#process method calls paramify_values on its parameters parameter. That method can modify the passed value (e.g. converting all values in a hash that are fixnums to strings). This behavior propagates to post, get and other methods that call process internally.

This behavior is unexpected and makes tests like this fail:

bug = { :id => 42, :title => "My bug" }

post :add_bug, :bug => bug

added_bug = Bug.find(:last)
assert_equal bug[:id],    added_bug.id    # Fails because bug[:id] is now "42", not 42.
assert_equal bug[:title], added_bug.title

The process method should probably duplicate parameters before modifying it.

Tested with Rails 3.1 RC5. Note this is a regression from Rails 2.3.x. (Not sure about 3.0.x.)

Owner

pixeltrix commented Aug 23, 2011

It looks like we just need to construct a new hash here.

pixeltrix closed this in 2af37b0 Aug 23, 2011

pixeltrix reopened this Aug 23, 2011

tenderlove was assigned Aug 23, 2011

Owner

pixeltrix commented Aug 23, 2011

@tenderlove can you backport this to the 3.1.0 branch, thanks.

pixeltrix closed this in 14cf4b2 Aug 24, 2011

pixeltrix reopened this Aug 25, 2011

Owner

pixeltrix commented Aug 25, 2011

Not sure why this got closed, but it still needs backporting to 3-1-0 as it's a regression from 3.0.x and could break existing tests.

/cc @tenderlove @spastorino

Owner

spastorino commented Aug 25, 2011

Done!

spastorino closed this Aug 25, 2011

pixeltrix closed this in 283f597 Aug 25, 2011

@gberenfield gberenfield pushed a commit to gberenfield/rails that referenced this issue Aug 30, 2011

@tenderlove tenderlove Merge branch '3-1-0' into 3-1-stable
* 3-1-0:
  Configuration changes for asset pipeline: remove config.assets.allow_debugging, add config.assets.compile and config.assets.digest
  Read digests of assets from manifest.yml if config.assets.manifest is on
  let SDoc add a link to the source code in GitHub for each method
  Documentation fixes
  bumping to 3.1.0.rc8
  bumping to 3.1.0.rc7
  Update Rails 3.1 CHANGELOGs
  Bump rack-cache, rack-test, rack-mount and sprockets up
  clear and disable query cache when an exception is raised from called middleware
  assert_no_match
  deletes spurious arrow
  use sdoc to generate the API
  Don't modify params in place - fixes #2624
  the command line guide is good to go
  `load` should also return the value from `super`
  require needs to return true or false. thank you Ryan "zenspider" Davis
  bumping bcrypt-ruby requirement
  Make ActionController::TestCase#recycle! set @protocol to nil
  Add failing test case for #2654

Conflicts:
	activerecord/test/cases/query_cache_test.rb
602a3c2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment