Why:
----
Following up on [#47323](https://github.com/rails/rails/issues/47323).
Many options are not forwarded to the Dalli client when it is
initialized from the `ActiveSupport::Cache::MemCacheStore`. This is to
support a broader set of features powered by the implementation. When an
instance of a client is passed on the initializer, it takes precedence,
and we have no control over which attributes will be overridden or
re-processed on the client side; this is by design and should remain as
such to allow both projects to progress independently. Having this
option introduces several potential bugs that are difficult to pinpoint
and get multiplied by which version of the tool is used and how each
evolves. During the conversation on the issue, the `Dalli` client
maintainer supports [deprecating](https://github.com/rails/rails/issues/47323#issuecomment-1424292456)
this option.
How:
----
Removing this implicit dependency will ensure each library can evolve
separately and cements the usage of `Dalli::Client` as an [implementation
detail](https://github.com/rails/rails/issues/21595#issuecomment-139815433)
We can not remove a supported feature overnight, so I propose we add a
deprecation warning for the next minor release(7.2 at this time).
There was a constant on the `Cache` namespace only used to restrict
options passed to the `Dalli::Client` initializer that now lives on the
`MemCacheStore` class.
Co-authored-by: Eileen M. Uchitelle <eileencodes@users.noreply.github.com>
Previously, requiring any of the ragel generated parsers in mail would
output tons of warnings in tests, making output much harder to read
(especially in Railties).
This commit extends the RaiseWarnings patch to suppress output from
these mail parsers.
The suppression can be removed once mikel/mail#1557 or mikel/mail#1551
or any other PR fixes the issue
* Remove Copyright years
* Basecamp is now 37signals... again
Co-authored-by: David Heinemeier Hansson <dhh@hey.com>
---------
Co-authored-by: David Heinemeier Hansson <dhh@hey.com>
Fix: https://github.com/rails/rails/issues/47418
We should trust the `Process._fork` callback, as it can't really
be bypassed except throught `Process.daemonize` but I don't think
it's a problem.
We initially kept the PID check even if the callback was present,
but it turns out that on modern GLIBC the getpid() syscall isn't
cached and it causes a significant overhead.
Add test coverage for existing `Object#with_options` support for
`Hash`-like objects.
The current implementation expects "Hash-like" to mean that the argument
implements both `Hash#deep_merge` and `#to_hash` (to be called
explicitly with `#to_hash` and implicitly with `**`).
There are multiple points of failure when processing a message with
`MessageEncryptor` or `MessageVerifier`, and there several ways we might
want to handle those failures. For example, swallowing a failure with
`MessageVerifier#verified`, or raising a specific exception with
`MessageVerifier#verify`, or conditionally ignoring a failure when
rotations are configured.
Prior to this commit, the _internal_ logic of handling failures was
implemented using a mix of `nil` return values and raised exceptions.
This commit reimplements the internal logic using `throw` and a few
precisely targeted `rescue`s. This accomplishes several things:
* Allow rotation of serializers for `MessageVerifier`. Previously,
errors from a `MessageVerifier`'s initial serializer were never
rescued. Thus, the serializer could not be rotated:
```ruby
old_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: Marshal)
new_verifier = ActiveSupport::MessageVerifier.new("secret", serializer: JSON)
new_verifier.rotate(serializer: Marshal)
message = old_verifier.generate("message")
new_verifier.verify(message)
# BEFORE:
# => raises JSON::ParserError
# AFTER:
# => "message"
```
* Allow rotation of serializers for `MessageEncryptor` when using a
non-standard initial serializer. Similar to `MessageVerifier`, the
serializer could not be rotated when the initial serializer raised an
error other than `TypeError` or `JSON::ParserError`, such as
`Psych::SyntaxError` or a custom error.
* Raise `MessageEncryptor::InvalidMessage` from `decrypt_and_verify`
regardless of cipher. Previously, when a `MessageEncryptor` was using
a non-AEAD cipher such as AES-256-CBC, a corrupt or tampered message
would raise `MessageVerifier::InvalidSignature` due to reliance on
`MessageVerifier` for verification. Now, the verification mechanism
is transparent to the user:
```ruby
encryptor = ActiveSupport::MessageEncryptor.new("x" * 32, cipher: "aes-256-gcm")
message = encryptor.encrypt_and_sign("message")
encryptor.decrypt_and_verify(message.next)
# => raises ActiveSupport::MessageEncryptor::InvalidMessage
encryptor = ActiveSupport::MessageEncryptor.new("x" * 32, cipher: "aes-256-cbc")
message = encryptor.encrypt_and_sign("message")
encryptor.decrypt_and_verify(message.next)
# BEFORE:
# => raises ActiveSupport::MessageVerifier::InvalidSignature
# AFTER:
# => raises ActiveSupport::MessageEncryptor::InvalidMessage
```
* Support `nil` original value when using `MessageVerifier#verify`.
Previously, `MessageVerifier#verify` did not work with `nil` original
values, though both `MessageVerifier#verified` and
`MessageEncryptor#decrypt_and_verify` do:
```ruby
encryptor = ActiveSupport::MessageEncryptor.new("x" * 32)
message = encryptor.encrypt_and_sign(nil)
encryptor.decrypt_and_verify(message)
# => nil
verifier = ActiveSupport::MessageVerifier.new("secret")
message = verifier.generate(nil)
verifier.verified(message)
# => nil
verifier.verify(message)
# BEFORE:
# => raises ActiveSupport::MessageVerifier::InvalidSignature
# AFTER:
# => nil
```
* Improve performance of verifying a message when it has expired and one
or more rotations have been configured:
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support/all"
verifier = ActiveSupport::MessageVerifier.new("new secret")
verifier.rotate("old secret")
message = verifier.generate({ "data" => "x" * 100 }, expires_at: 1.day.ago)
Benchmark.ips do |x|
x.report("expired message") do
verifier.verified(message)
end
end
```
__Before__
```
Warming up --------------------------------------
expired message 1.442k i/100ms
Calculating -------------------------------------
expired message 14.403k (± 1.7%) i/s - 72.100k in 5.007382s
```
__After__
```
Warming up --------------------------------------
expired message 1.995k i/100ms
Calculating -------------------------------------
expired message 19.992k (± 2.0%) i/s - 101.745k in 5.091421s
```
Fixes#47185.
This factors common methods into a `ActiveSupport::Messages::Codec` base
class. This also disentangles serialization (and deserialization) from
encryption (and decryption) in `MessageEncryptor`.
Prior to this commit, messages with metadata were always serialized in
the following way:
```ruby
Base64.strict_encode64(
ActiveSupport::JSON.encode({
"_rails" => {
"message" => Base64.strict_encode64(
serializer.dump(data)
),
"pur" => "the purpose",
"exp" => "the expiration"
},
})
)
```
in which the message data is serialized and URL-encoded twice.
This commit changes message serialization such that, when possible, the
data is serialized and URL-encoded only once:
```ruby
Base64.strict_encode64(
serializer.dump({
"_rails" => {
"data" => data,
"pur" => "the purpose",
"exp" => "the expiration"
},
})
)
```
This improves performance in proportion to the size of the data:
**Benchmark**
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support/all"
verifier = ActiveSupport::MessageVerifier.new("secret", serializer: JSON)
payloads = [
{ "content" => "x" * 100 },
{ "content" => "x" * 2000 },
{ "content" => "x" * 1_000_000 },
]
if ActiveSupport::Messages::Metadata.respond_to?(:use_message_serializer_for_metadata)
ActiveSupport::Messages::Metadata.use_message_serializer_for_metadata = true
end
Benchmark.ips do |x|
payloads.each do |payload|
x.report("generate ~#{payload["content"].size}B") do
$generated_message = verifier.generate(payload, purpose: "x")
end
x.report("verify ~#{payload["content"].size}B") do
verifier.verify($generated_message, purpose: "x")
end
end
end
puts
puts "Message size:"
payloads.each do |payload|
puts " ~#{payload["content"].size} bytes of data => " \
"#{verifier.generate(payload, purpose: "x").size} byte message"
end
```
**Before**
```
Warming up --------------------------------------
generate ~100B 1.578k i/100ms
verify ~100B 2.506k i/100ms
generate ~2000B 447.000 i/100ms
verify ~2000B 1.409k i/100ms
generate ~1000000B 1.000 i/100ms
verify ~1000000B 6.000 i/100ms
Calculating -------------------------------------
generate ~100B 15.807k (± 1.8%) i/s - 80.478k in 5.093161s
verify ~100B 25.240k (± 2.1%) i/s - 127.806k in 5.066096s
generate ~2000B 4.530k (± 2.4%) i/s - 22.797k in 5.035398s
verify ~2000B 14.136k (± 2.3%) i/s - 71.859k in 5.086267s
generate ~1000000B 11.673 (± 0.0%) i/s - 59.000 in 5.060598s
verify ~1000000B 64.372 (± 6.2%) i/s - 324.000 in 5.053304s
Message size:
~100 bytes of data => 306 byte message
~2000 bytes of data => 3690 byte message
~1000000 bytes of data => 1777906 byte message
```
**After**
```
Warming up --------------------------------------
generate ~100B 4.689k i/100ms
verify ~100B 3.183k i/100ms
generate ~2000B 2.722k i/100ms
verify ~2000B 2.066k i/100ms
generate ~1000000B 12.000 i/100ms
verify ~1000000B 11.000 i/100ms
Calculating -------------------------------------
generate ~100B 46.984k (± 1.2%) i/s - 239.139k in 5.090540s
verify ~100B 32.043k (± 1.2%) i/s - 162.333k in 5.066903s
generate ~2000B 27.163k (± 1.2%) i/s - 136.100k in 5.011254s
verify ~2000B 20.726k (± 1.7%) i/s - 105.366k in 5.085442s
generate ~1000000B 125.600 (± 1.6%) i/s - 636.000 in 5.064607s
verify ~1000000B 122.039 (± 4.1%) i/s - 616.000 in 5.058386s
Message size:
~100 bytes of data => 234 byte message
~2000 bytes of data => 2770 byte message
~1000000 bytes of data => 1333434 byte message
```
This optimization is only applied for recognized serializers that are
capable of serializing a `Hash`.
Additionally, because the optimization changes the message format, a
`config.active_support.use_message_serializer_for_metadata` option has
been added to disable it. The optimization is disabled by default, but
enabled with `config.load_defaults 7.1`.
Regardless of whether the optimization is enabled, messages using either
format can still be read.
In the case of a rolling deploy of a Rails upgrade, wherein servers that
have not yet been upgraded must be able to read messages from upgraded
servers, the optimization can be disabled on first deploy, then safely
enabled on a subsequent deploy.
Prior to this commit, `ActiveSupport::SecureCompareRotator` used
`ActiveSupport::Messages::Rotator` for part of its rotation logic, even
though `SecureCompareRotator` is entirely unrelated to messages. This
made it precarious to alter `Messages::Rotator`, especially because
`Messages::Rotator` was `prepend`ed to `SecureCompareRotator` rather
than `include`d.
This commit reimplements `SecureCompareRotator` without
`Messages::Rotator`, which simplifies the logic and, as a bonus,
improves performance:
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support/all"
comparer = ActiveSupport::SecureCompareRotator.new("new secret")
comparer.rotate("old secret")
Benchmark.ips do |x|
x.report("compare old") do
comparer.secure_compare!("old secret")
end
end
```
__Before__
```
Warming up --------------------------------------
compare old 72.073k i/100ms
Calculating -------------------------------------
compare old 719.844k (± 1.0%) i/s - 3.604M in 5.006682s
```
__After__
```
Warming up --------------------------------------
compare old 147.486k i/100ms
Calculating -------------------------------------
compare old 1.473M (± 0.9%) i/s - 7.374M in 5.006655s
```
Add as_json method for the new Data object introduced in Ruby 3.2 so
Data objects can be encoded/decoded.
data = Data.define(:name).new(:test)
data.as_json
We calculate idle time by subtracting two separately-obtained values,
and apparently it's possible for the CPU-time clock to tick a
millisecond ahead of the monotonic clock.
I think it's safe to ignore that potential difference in general, but
even at the extreme of CPU activity, I'm reasonably confident time
doesn't move backwards.
Follow-up to #42846.
Prior to this commit, `ActiveSupport::EncryptedFile` used the
`ActiveSupport::MessageEncryptor` default serializer. #42846 changed
the default serializer to `JSON` when using `config.load_defaults 7.1`.
Thus, encrypted files that were written with the previous default
serializer (`Marshal`) could not be read by `Rails.application.encrypted`.
That included files which were written with `bin/rails encrypted:edit`
even when using the new default, because the `bin/rails encrypted:edit`
command does not run the initializer that applies the configuration to
`MessageEncryptor`:
```console
$ bin/rails r 'puts ActiveSupport::MessageEncryptor.default_message_encryptor_serializer'
json
$ bin/rails encrypted:edit config/my_encrypted.yml.enc
...
$ bin/rails encrypted:show config/my_encrypted.yml.enc
foo: bar
$ bin/rails r 'Rails.application.encrypted("config/my_encrypted.yml.enc").read'
rails/activesupport/lib/active_support/message_encryptor.rb:241:in `rescue in _decrypt': ActiveSupport::MessageEncryptor::InvalidMessage (ActiveSupport::MessageEncryptor::InvalidMessage)
...
ruby-3.2.0/lib/ruby/3.2.0/json/common.rb:216:in `parse': unexpected token at "foo: bar (JSON::ParserError)
```
This commit hard codes the serializer for `EncryptedFile` to `Marshal`.
Ruby 3.2 significantly changed how instance variables are store.
It now use shapes, and in short, it's important for performance
to define instance variables in a consistent order to limit the
amount of shapes.
Otherwise, the number of shapes will increase past a point where
MRI won't be able to cache instance variable access. The impact
is even more important when YJIT is enabled.
This PR is data driven. I dump the list of Shapes from Shopify's
monolith production environment, and Rails is very present among
the top offenders:
```
Shape Edges Report
-----------------------------------
770 @default_graphql_name
697 @own_fields
661 @to_non_null_type
555 @own_interface_type_memberships
472 @description
389 @errors
348 @oseid
316 @_view_runtime
310 @_db_runtime
292 @visibility
286 @shop
271 @attribute_method_patterns_cache
264 @namespace_for_serializer
254 @locking_column
254 @primary_key
253 @validation_context
244 @quoted_primary_key
238 @access_controls
234 @_trigger_destroy_callback
226 @_trigger_update_callback
224 @finder_needs_type_condition
215 @_committed_already_called
214 @api_type
203 @mutations_before_last_save
202 @access_controls_overrides
201 @options
198 @mutations_from_database
190 @_already_called
183 @name
179 @_request
176 @own_arguments
175 @_assigns
175 @virtual_path
174 @context
173 @_controller
173 @output_buffer
173 @view_flow
172 @_default_form_builder
169 @cache
159 @_touch_record
151 @attribute_names
151 @default_attributes
150 @columns_hash
149 @attribute_types
148 @columns
147 @marked_for_same_origin_verification
146 @schema_loaded
143 @_config
143 @type
141 @column_names
```
All the changes are of similar nature, the goal is to preset the instance
variable to nil when objects are allocated, or when classes are created.
For classes I leverage the `inherited` hook. If the patern becomes common enough
it might make sense to add a helper for this in `ActiveSupport::Concern`.
Attributes such as `set` and `reset` should not be used as they clash with the `CurrentAttributes` public API. This PR raises an `ArgumentError` if a restricted attribute name is used.
A user reported seeing the following error when trying to use the
EventedFileUpdateChecker in their app:
```shell
$ rails server
=> Booting Puma
=> Rails 7.0.4 application starting in development
=> Run `bin/rails server --help` for more startup options
Exiting
/usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require': cannot load such file -- listen (LoadError)
from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
from /usr/local/lib/ruby/gems/3.0.0/gems/activesupport-7.0.4/lib/active_support/evented_file_update_checker.rb:6:in `<top (required)>'
from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
from /usr/local/lib/ruby/gems/3.0.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
from /home/ross/Data/EmySystem/newentry/Entry2/config/environments/development.rb:84:in `block in <top (required)>'
from /usr/local/lib/ruby/gems/3.0.0/gems/railties-7.0.4/lib/rails/railtie.rb:257:in `instance_eval'
from /usr/local/lib/ruby/gems/3.0.0/gems/railties-7.0.4/lib/rails/railtie.rb:257:in `configure'
from /home/ross/Data/EmySystem/newentry/Entry2/config/environments/development.rb:3:in `<top (required)>'
...
```
While we were able to direct them to the proper fix (add listen to
their app's Gemfile), the error message does not really point them in
that direction on its own.
This commit improves the error message by using Kernel#gem to indicate
that the gem should be in the user's bundle. This is the same approach
used for other not-specified dependencies in Rails, such as the various
database drivers in their Active Record adapters.
New error message:
```shell
$ bin/rails r "ActiveSupport::EventedFileUpdateChecker"
/home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/site_ruby/3.2.0/bundler/rubygems_integration.rb:276:in `block (2 levels) in replace_gem': listen is not part of the bundle. Add it to your Gemfile. (Gem::LoadError)
from /home/hartley/src/github.com/skipkayhil/rails/activesupport/lib/active_support/evented_file_update_checker.rb:3:in `<top (required)>'
from /home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
from /home/hartley/.cache/asdf/installs/ruby/3.2.0/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
...
```
Ref: ad39d6b
Ensure that all evals include file and line number to identify their
source.
Two of the evals reported by this cop were unneccesary and replaced with
non-eval alternatives: xml is set as a local variable in
Template::Handlers::Builder#call, and instance_eval isn't needed to get
the current binding.
There are additionally 27 offenses in test directories, but since it
seems less important to fix those they are currently ignored.
Active Support's original Hash#transform_keys used not to take any argument, and
that version has been transported to Ruby 2.5
https://github.com/ruby/ruby/commit/1405111722
Then since Ruby 3.0, that method in Ruby has been expanded to take a Hash
argument https://github.com/ruby/ruby/commit/b25e27277d
Hence, we'd better update our own in-house Hash-like class
HashWithIndifferentAccess to follow the Hash API change.
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
