ActiveSupport::MessageEncryptor migration path to 7.1 defaults

[etiennebarrie]

Steps to reproduce

• With a 7.0 app, generate a encrypted message with ActiveSupport::MessageEncryptor.
• Upgrade the app 7.1 keeping 7.0 defaults: you can decode the message, generate a new message.
• Use the 7.1 defaults: you can't decode the message.

I created a small example app which you can clone with git clone https://github.com/etiennebarrie/rails-app.git --single-branch --branch message-encryptor-7.1-compat. With this app, the steps are:

$ bundle
$ bin/rails server
# navigate to http://127.0.0.1:3000/
# optionally click "Verify" to ensure the message roundtrip works on 7.0
# keep the window open to verify a 7.0 message with 7.1
$ BUNDLE_GEMFILE=Gemfile.main bundle
$ BUNDLE_GEMFILE=Gemfile.main bin/rails s
# now click "Verify" to verify the 7.0 generated message with 7.1 with 7.0 defaults
# it works!
# keep the window open
$ RAILS_71_DEFAULTS=1 BUNDLE_GEMFILE=Gemfile.main bin/rails s
# click "Verify"
# verification failed

I know there are more steps mentioned in https://edgeguides.rubyonrails.org/upgrading_ruby_on_rails.html, but out-of-the-box this should work. Instead multiple steps are necessary.

Expected behavior

The application is upgraded and the message should be verified.

Actual behavior

Message verification fails.

---

For each line in these tables, you should check what is generated and the fact that it is verifiable by the line above and the line below.
The approach right now can be resumed to this:

Application configuration	Generation	Can verify Marshal	Can verify JSON
7.0	Marshal	✓	𐄂
7.1 with 7.0 defaults	Marshal	✓	𐄂
7.1 with 7.1 defaults	JSON	𐄂	✓

We can see the incompatibility by looking at what each steps generates and what each next step can verify. The step 2 to 3 is incompatible.

The approach recommended in the upgrading guide:

Application configuration	Generation	Can verify Marshal	Can verify JSON
7.0	Marshal	✓	𐄂
7.1 with 7.0 defaults, default_message_encryptor_serializer = :marshal	Marshal	✓	𐄂
7.1 with 7.0 defaults, default_message_encryptor_serializer = :hybrid	Marshal	✓	✓
7.1 with 7.1 defaults, default_message_encryptor_serializer = :hybrid, use_marshal_serialization = false	JSON	✓	✓
7.1 with 7.1 defaults	JSON	𐄂	✓

This shows it's compatible, but adds multiple steps to the upgrade. The second step is not necessary, but still described in the upgrading guide.

What I think we should do, in line with the way we usually use defaults:

Application configuration	Generation	Can verify Marshal	Can verify JSON
7.0	Marshal	✓	𐄂
7.1 with 7.0 defaults	Marshal	✓	✓
7.1 with 7.1 defaults	JSON	✓	✓
7.1 with 7.1 defaults, default_message_encryptor_serializer = :json	JSON	𐄂	✓

That last line, with default_message_encryptor_serializer = :json can become the default in 7.2, and can be enabled earlier to make sure no Marshal is accepted before then.

[jonathanhefner]

This issue also applies to ActiveSupport::MessageVerifier / config.active_support.default_message_verifier_serializer.

#47963 will not fix this issue, but I think it is a step in the right direction. If #47963 is merged, there will be a single setting (config.active_support.message_serializer) that can default to :json_allow_marshal in 7.1, and provide the described behaviors for both MessageEncryptor and MessageVerifier. Then, as you said, we can change the default to :json in 7.2.