Worth an issue? Possible "bug" in ActionDispatch::Request::Session

rubyonrails-core
1

We’re given a couple of methods to modify the ActionDispatch::Request::Session object. All work as expected except merge!:

session.to_hash
#=> {"session_id"=>"abc123"}
session.update({ foo: "bar" })
#=> {"session_id"=>"abc123", "foo" => "bar"}
session[:foo]
#=> "bar"
session[:bar] = "baz"
#=> {"session_id"=>"abc123", "foo" => "bar", "bar" => "baz"}
session[:bar]
#=> "baz"
session.merge!({ fizz: "buzz" })
#=> {"session_id"=>"abc123", "foo" => "bar", "bar" => "baz", fizz: "buzz"}
session[:fizz]
#=> nil

While []= and update stringify the key before updating the “hash”, merge! does not. The session is properly loaded with stringified keys on the next request but within the request that the session is updated attempting to access by key will fail.

I hesitate to open an issue or a PR for this because it’s so minor and because on subsequent request the key is a string as expected. However, I thought I’d put it up on here and see if anyone else thought it was worth a PR. It’s a fairly simple “fix” but I dont want to waste the core teams time reviewing my PR if no one see it as an issue or worth it.

1 Like
2

I think this can definitely be a pull request, seems like merge! needs to be overwritten but not sure

3

Should be as simple as changing this:

def merge!(other)
  load_for_write!
  @delegate.merge!(other)
end

to

def merge!(other)
  load_for_write!
  @delegate.merge!(other.stringify_keys)
end

:man_shrugging:

4

PR is opened, Stringify keys of `other` hash when performing a merge by DRBragg · Pull Request #44261 · rails/rails · GitHub

Testing through me for a loop but I think it all makes sense :sweat_smile: