search

rack / rack-session

MIT License
32 stars 14 forks source link

Allow the v2 encryptor to serialize messages with `Marshal` #44

Open jcmfernandes opened 1 month ago

jcmfernandes commented 1 month ago

This is a follow-up on #39 that must be merged first.

I deliberately avoided allowing Marshal serialization in the v2 encryptor because 1) Marshal has been a source of RCE vulnerabilities and 2) serializing messages as JSON allows for better interoperability, making it trivial to access sessions created with rack-session in other languages.

However, it's impossible to swap Marhal with JSON without breaking users' code that relies on Marhal behavior that JSON doesn't mimic (e.g., preserving ruby symbols).

So... I see 3 ways forward:

1) We close this PR and release yet another major 2) We close this PR and make v2 opt-in instead of the default 3) We merge this PR

Happy to hear some feedback.

ioquatix commented 1 month ago

I've merger your other PR.

I'm a big fan of symbolic keys if the keys don't represent arbitrary user data, so I'd support the JSON decoder using symbolize_names or similar approaches.

I think we can gracefully introduce support using a flag or option, or deprecation warnings, or as you say, a major version bump.

Do you mind rebasing on main?