Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

write_cookies! in ActionDispatch::TestRequest won't write nil value #2069

Closed
bfolkens opened this Issue · 5 comments

2 participants

@bfolkens

Rack::Utils.escape(str) breaks when cookie value is nil.

NoMethodError: private method `gsub' called for nil:NilClass
    activesupport (3.1.0.rc4) lib/active_support/whiny_nil.rb:48:in `method_missing'
    rack (1.3.1) lib/rack/backports/uri/common.rb:24:in `encode_www_form_component'
    rack (1.3.1) lib/rack/utils.rb:23:in `escape'
    actionpack (3.1.0.rc4) lib/action_dispatch/testing/test_request.rb:86:in `escape_cookie'
    actionpack (3.1.0.rc4) lib/action_dispatch/testing/test_request.rb:81:in `write_cookies!'
@bfolkens

Patched with

--- a/lib/action_dispatch/testing/test_request.rb   2011-06-23 14:05:38.000000000 -0500
+++ b/lib/action_dispatch/testing/test_request.rb   2011-07-14 09:03:51.000000000 -0500
@@ -78,7 +78,7 @@
     private
       def write_cookies!
         unless @cookies.blank?
-          @env['HTTP_COOKIE'] = @cookies.map { |name, value| escape_cookie(name, value) }.join('; ')
+          @env['HTTP_COOKIE'] = @cookies.map { |name, value| value && escape_cookie(name, value) }.join('; ')
         end
       end
@pixeltrix pixeltrix was assigned
@pixeltrix
Owner

On master this shouldn't be a problem as we no longer write out the cookie header - in this case it's probably better to coerce the value to a string in escape_cookie.

@bfolkens

Will do - I'll change the patch and make a pull request?

@bfolkens bfolkens referenced this issue from a commit in bfolkens/rails
@bfolkens bfolkens Fix for issue #2069: write_cookies! in ActionDispatch::TestRequest wo…
…n't write nil value
d895104
@pixeltrix
Owner

A test would be good as well :-)

@bfolkens

Just saw this after making the pull request - I'll switch this over to there for discussion.

@bfolkens bfolkens closed this
@bfolkens bfolkens referenced this issue from a commit in bfolkens/rails
@bfolkens bfolkens Added test case for issue #2069 560f68a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.