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

JSON serialization doesn't support UTF-8 #38

Open
iain opened this issue Feb 1, 2024 · 2 comments · May be fixed by #39
Open

JSON serialization doesn't support UTF-8 #38

iain opened this issue Feb 1, 2024 · 2 comments · May be fixed by #39

Comments

@iain
Copy link

iain commented Feb 1, 2024

Hi, I was trying to use the JSON serializer and ran into trouble when trying to put an emoji into the session (in real life, this is more likely to happen with flash messages).

Here is the code to reproduce it:

require 'rack/session'

use Rack::Session::Cookie, secrets: ('a' * 64), serialize_json: true

run lambda { |env|
  env[Rack::RACK_SESSION][:foo] = '😀'
  [200, {}, ['']]
}

The output is as follows:

incompatible character encodings: ASCII-8BIT and UTF-8 (Encoding::CompatibilityError)
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/encryptor.rb:177:in `serialize_payload'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/encryptor.rb:105:in `encrypt'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/cookie.rb:296:in `encode_session_data'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/cookie.rb:267:in `write_session'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/abstract/id.rb:394:in `commit_session'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/abstract/id.rb:274:in `context'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/rack-session-2.0.0/lib/rack/session/abstract/id.rb:266:in `call'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/configuration.rb:272:in `call'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/request.rb:100:in `block in handle_request'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/thread_pool.rb:378:in `with_force_shutdown'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/request.rb:99:in `handle_request'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/server.rb:464:in `process_client'
/Users/iain/.asdf/installs/ruby/3.3.0/lib/ruby/gems/3.3.0/gems/puma-6.4.2/lib/puma/server.rb:245:in `block in run'

It looks like the problem arises from the padding in the encryptor. The padding amount and random bytes are not compatible with the json output, and I don't see an easy way around it either.

Of course the default Marshal encoding works, but I was hoping to move away from that.

@ioquatix
Copy link
Member

ioquatix commented Feb 1, 2024

Does the JSON data get encoded as the raw value of the header? If so, the only solution would be to base64 (or url encode) it, e.g. emoji -> JSON -> url encoded header. IF that's not what we are already doing, I'd accept a PR for that.

@jcmfernandes
Copy link
Contributor

Forcing the encoding of serialized_data in Rack::Session::Encryptor::Serializable#serialize_payload to BINARY fixes it; something we should do, as we want to send it as-is. I'll prepare a PR over the weekend, adding tests to cover this scenario. Thanks for the report @iain!

jcmfernandes added a commit to jcmfernandes/rack-session that referenced this issue Feb 13, 2024
jcmfernandes added a commit to jcmfernandes/rack-session that referenced this issue Feb 13, 2024
jcmfernandes added a commit to jcmfernandes/rack-session that referenced this issue Feb 13, 2024
@jcmfernandes jcmfernandes linked a pull request Feb 13, 2024 that will close this issue
jcmfernandes added a commit to jcmfernandes/rack-session that referenced this issue Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants