Given that the limiter implementation provided by Kredis is a simple
increment with a limit, all `ActiveSupport::Cache` already provide that
same capability, with a wide range of backing stores, and not just Redis.
This even allow to use SolidCache has a backend if you so desire.
If we feel particularly fancy, we could also accept a more generic
limiter interface to better allow users to swap the implementation
for better algorithms such as leaky-bucket etc.
Previously, it was theoretically possible to define a route with a
Symbol as a `to:` value (or at least, it would not raise a
`NoMethodError`). However, passing a Symbol broke when `/#/.match?(to)`
was [replaced][1] with `to&.include?("#")` with the assumption that `to`
was always a String.
Instead of restoring the previous error, this commit improves how the
`to:` value is checked so that it raises an `ArgumentError` for any
invalid values. The extra strictness will specifically improve the error
when a Symbol or String that doesn't include a "#" are passed since they
were effectively equivalent to passing a `nil` value, or not specifying
`to:` at all.
[1]: 5726b1d1d7665c33830ad114f45258ecf772b7b6
While working on [another PR][1], I found that removing the
Transfer-Encoding conditionals did not result in any failing tests in
Action Pack. This was surprising to me until I found that there was a
test for this behavior in Railties. However, nothing about the test
really depends on having a full Rails application or the Railties test
suite.
This commit moves the test into Action Pack to simplify/speedup the test
(no need to build a full app) as well as keeping the test closer to the
actual behavior being tested.
[1]: 0c334b48fdc5d70b0c8406ba184fbfd26750b049
We recently had [two][1] [PRs][2] to update these types of links, so
this commit does all of the rest (remaining links to master branches
were checked and still exist).
[1]: e76c52a939dd7312fb9fd22b08e13496dc5c961e
[2]: a2ed3437e3be1e4dee4d1dac3c51a3b8742044ff
Provide examples for rendering objects that respond to `render_in`. Also
highlight that the object can also define a `#format` method to control
how the rendered String should be treated.
Add test coverage for both Action View's and Action Pack's support for
`render` with `:renderable` options.
Now that we dropped support for Ruby 2.7, we no longer
need to check if variables are defined before accessing them
to avoid the undefined variable warning.
Now that we no longer support Ruby 2.7, many `ruby2_keyword` calls
can be eliminated.
The ones that are left could be eliminated but would end up substantially
slower or more compliacated so I left them for now.
Also perform two autocorrects with `bundle exec rubocop -A`:
- fixes a new case of [`Style/RedundantReturn`][1]
- fixes a new case of [`Performance/StringInclude`][2]
[1]: 146b1c2e3389bc70ea0b54abf7843fc1d6c8cd5f
[2]: 3158bbb9f6454dce64dd0b4e2a548351d014c48f
Co-authored-by: David Heinemeier Hansson <david@basecamp.com>
Until now, Rails only droped compatibility with older
rubies on new majors, but I propose to change this policy
because it causes us to either keep compatibility with long
EOLed rubies or to bump the Rails major more often, and to
drop multiple Ruby versions at once when we bump the major.
In my opinion it's a bad alignments of incentives. And we'd
be much better to just drop support in new minors whenever they
go EOL (so 3 years).
Also Ruby being an upstream dependency, it's not even
a semver violation AFAICT.
Since Rails 7.2 isn't planned before a few months, we
can already drop Ruby 3.0 as it will be EOL in March.
Follow-up to #46530.
The dynamically generated `url_helpers` module is an
`ActiveSupport::Concern`. Therefore, when it is included directly in
another `ActiveSupport::Concern`, its `included` block is deferred until
the latter concern is itself included elsewhere. Thus, in that case,
the call to `base._routes` in `def self.included(base)` will raise
`NoMethodError` because the `included` block will not yet have defined
the `_routes` method.
This commit prevents the error by first checking if `base` responds to
`_routes`.
Rack was recently [updated][1] with a deprecation for some status codes
that have been renamed (most notably, Unprocessable Entity was renamed
to Unprocessable Content). Since the deprecation was only added to
the `#status_code` method, this has caused test failures for some tests
that depend on the `SYMBOLS_TO_STATUS_CODE` hash.
This commit replaces the usage of `SYMBOLS_TO_STATUS_CODE` with
`#status_code` so that we get the deprecation message instead of a test
failure.
[1]: 64ad26e338
Follow-up to #49908.
When Selenium resolves the driver path to a copy of Chrome that it has
downloaded / cached, it mutates the `Selenium::WebDriver::Chrome::Options`
object it receives, and relies on those changes later when the options
are used. If `Selenium::WebDriver::Chrome::Service.driver_path` is set
but a different options object is used, Selenium will raise "cannot find
Chrome binary". Therefore, this commit ensures that the options object
passed to `Selenium::WebDriver::DriverFinder.path` is the same options
object used by the driver later.
method cause an Internal Server Error due to a TypeError.
In order to display the extraced source of a syntax error, we try
to locate the node id for the backtrace location. The call to
RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location is expecting
a Thread::Backtrace::Location object, but we are passing a
SourceMapLocation instead.
This commit does two things:
1) it addresses the issue by making sure that we are always passing
a Thread::Backtrace::Location instead
2) it allows the development view to show the extracted source fragment
We can't run this test on Ruby 2.7 due to minitest being locked in the
Gemfile to an older version, so we should use that as a condition
instead of skipping if minitest doesn't have metadata.
Adds support for with_routing test helpers in ActionDispatch::IntegrationTest.
Previously, this helper didn't work in an integration context because
the rack app and integration session under test were not mutated.
Because controller tests are integration tests by default, we should
support test routes for these kinds of test cases as well.
This cache is used when url_for is called without a named route (ie.
when it's called with hash options). Eager loading avoids building the
cache on the first call and potentially allows the memory to be shared
via CoW on forking servers.
When the webdrivers gem is not present (which is the default scenario in
Rails 7.1+), the Selenium `driver_path` starts out as `nil`. This means
the driver is located lazily, and deferred until a system test is run.
If parallel testing is used, this leads to a race condition, where each
worker process tries to resolve the driver simultaneously. The result is
an error as described in #49906.
This commit fixes the race condition by changing the implementation of
`Browser#preload`. The previous implementation worked when `driver_path`
was set to a Proc by the `webdrivers` gem, but doesn't work when the
`webdrivers` gem is not being used and the `driver_path` is `nil`.
`Browser#preload` now uses the `DriverFinder` utility provided by the
`selenium-webdriver` gem to eagerly resolve the driver path if needed.
This will ensures that `driver_path` is set before parallel test workers
are forked.
Fixes#49906.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Previously, both the `@rendered_format` and
`@marked_for_same_origin_verification` instance variables would be
assigned to instances of `ActionView::Base`, making them accessible in
view templates. However, these instance variables are really internal to
the controller and result in extra string allocations because the `@`
gets stripped and readded when going through the assignment.
This commit prefixes the variables with an underscore to help indicate
that they are internal, and then adds them to the list of
`_protected_ivars` to prevent assigning them when rendering templates.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
For better or worse, the Rails guide settled on double quotes
and a large part of the community also use rubocop which enforce
them by default.
So we might as well try to follow that style when providing code
snippets in the documentation or error messages.
Fix: https://github.com/rails/rails/issues/49822
I certainly didn't get them all, but consistency should be significantly
improved.
Ruby 3.3.0 is going to start warning for racc not being specififed as a
dependency, and Ruby 3.4.0 will raise if it is not specified.
This commit prevents those issues by adding racc to the Action Pack
gemspec, since `racc/parser` is a runtime dependency of the Journey
parser.
It used to be stdlib but is being extracted in modern rubies.
Overall its usefulness is dubious. In all cases it is included in
Rails, it's only for the `synchronize` method, but end up exposing
a dozen other useless methods.
In the end just using a Mutex is clearer and simpler.
In some cases we can even get away with a single mutex in a constant.
This code was introduced by #17221 to workaround issues with not having
the `:en` locale set in the app to translate when calling `to_sentence`,
when having `I18n.enforce_available_locales` enabled.
We can still use the helper, with the defaults provided by the code,
without using I18n and thus without relying on the app locale, by
passing the `locale: false` option.
The downside to this is that we cannot generate ETags for these types of responses, but are assuming that by using an enumerator they don't expect a buffered response to be cacheable. This means you cannot use Enumerator to generate streaming responses.
Fixes#49588
See also: #47092
Co-authored-by: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Follow-up to [#45369][]
First, add the final `:` to the `#deep_merge?` method's `:nodoc:`
declaration.
Next, move the `:method:` documentation out of the methods and into the
section of the class that defines the rest of the dynamic method
documentation.
Finally, move the `:call-seq:` methods below the directive, like the
rest of the methods.
[#45369]: https://github.com/rails/rails/pull/45369
It's possible since Rails 6 (3ea2857943dc294d7809930b4cc5b318b9c39577) to let the framework create Event objects, but the guides and docs weren't updated to lead with this example.
Manually instantiating an Event doesn't record CPU time and allocations, I've seen it more than once that people copy-pasting the example code get confused about these stats returning 0. The tests here show that - just like the apps I've worked on - the old pattern keeps getting copy-pasted.
This was [added][1] when the default configuration was added for Rails
5.2, however the accessor itself has never been documented or used.
`protect_from_forgery: :exception` is added based on whether the
configuration is set on `config.action_controller` and not this value.
Since the accessor is undocumented and unused, this commit removes it.
[1]: 48cb8b3e7097e9a1cb45b2298f59b9179f0dbdee
When [rails/rails#20868][] changed the `ActionController::Parameters`
ancestory from `HashWithIndifferentAccess` to `Object`, support for
`#deep_merge` and `#deep_merge!` were omitted.
This commit restores support by integrating with
[ActiveSupport::DeepMergeable](./activesupport/lib/active_support/deep_mergeable.rb).
[rails/rails#20868]: https://github.com/rails/rails/pull/20868
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Thanks to https://github.com/rails/rails/pull/29742,
newly created Rails apps after Rails 5.2 don't have to deep dive into
CSRF problems, and they don't require direct call of
`protect_from_forgery`. However, some projects sill have to consider the
case where `protect_from_forgery` should be called.
Calling `protect_from_forgery` without `:with` option treats `:with` as
`:null_session` by default, not `:exception`, so I was a bit confused by
the inconsistency between `default_protect_from_forgery` and
`protect_from_forgery`. Maybe, changing the default `:with` to
`:exception` will bring significant breaking changes,
so I want to suggest adding a notice to the method.
There are assertions that expected/actual arguments are passed in the
reversed order by mistake. Enabling the LiteralAsActualArgument rule
prevents this mistake from happening.
The existing tests were auto-corrected by rubocop with a bit of
indentation adjustment.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Introduced in https://github.com/rails/rails/pull/49042, the method `ActionController::Parameters#extract_value` promises to replace utility methods that were previously defined as private methods in controllers.
However, it currently throws a `NoMethodError` when passed a non-existent key.
`params` is dependent on client requests and is thus beyond the application's control.
Rather than throwing a `NoMethodError`, it would be more convenient for the method to return `nil`.
This commit adds `extract_value` method to `ActionController::Parameters`
as a primary way to extract composite `id` values serialized from
`ActiveRecord::Base#to_param` called on a model with a composite primary key.
I was benchmarking some specs in my app, and saw this code come up in the
memory_profiler. It is by no means the biggest memory allocation, but it
is straightforward to make a slight improvement.
Primarily, this saves allocating one array by using concat instead of +
to add public_instance_methods(false).
That ends up being 10% less memory for my benchmark (3 actions), and 6%
faster.
```
Calculating -------------------------------------
original 8.352k memsize ( 208.000 retained)
22.000 objects ( 2.000 retained)
6.000 strings ( 0.000 retained)
refactored3 7.616k memsize ( 408.000 retained)
11.000 objects ( 7.000 retained)
3.000 strings ( 3.000 retained)
Comparison:
refactored3: 7616 allocated
original: 8352 allocated - 1.10x more
Warming up --------------------------------------
original 2.326k i/100ms
refactored3 2.441k i/100ms
Calculating -------------------------------------
original 23.336k (± 0.7%) i/s - 118.626k in 5.083658s
refactored3 24.692k (± 1.2%) i/s - 124.491k in 5.042345s
Comparison:
original: 23336.0 i/s
refactored3: 24692.5 i/s - 1.06x faster
```
Benchmark and results are also posted to https://gist.github.com/technicalpickles/4a4ae6a9e2c42963af43a89f75e768fe
Both `Nokogiri` and `Minitest` have merged the PRs mentioned to
integrate support for Ruby's Pattern matching
(https://github.com/sparklemotion/nokogiri/pull/2523 and
https://github.com/minitest/minitest/pull/936, respectively).
This commit adds coverage for those new assertions, and incorporates
examples into the documentation for the `response.parsed_body` method.
In order to incorporate pattern-matching support for JSON responses,
this commit changes the response parser to call `JSON.parse` with
[object_class: ActiveSupport::HashWithIndifferentAccess][object_class],
since String instances for `Hash` keys are incompatible with Ruby's
syntactically pattern matching.
For example:
```ruby
irb(main):001:0> json = {"key" => "value"}
=> {"key"=>"value"}
irb(main):002:0> json in {key: /value/}
=> false
irb(main):001:0> json = {"key" => "value"}
=> {"key"=>"value"}
irb(main):002:0> json in {"key" => /value/}
.../3.2.0/lib/ruby/gems/3.2.0/gems/irb-1.7.4/lib/irb/workspace.rb:113:in `eval': (irb):2: syntax error, unexpected terminator, expecting literal content or tSTRING_DBEG or tSTRING_DVAR or tLABEL_END (SyntaxError)
json in {"key" => /value/}
^
.../ruby/3.2.0/lib/ruby/gems/3.2.0/gems/irb-1.7.4/exe/irb:9:in `<top (required)>'
.../ruby/3.2.0/bin/irb:25:in `load'
.../ruby/3.2.0/bin/irb:25:in `<main>'
```
When the Hash maps String keys to Symbol keys, it's able to be pattern
matched:
```ruby
irb(main):005:0> json = {"key" => "value"}.with_indifferent_access
=> {"key"=>"value"}
irb(main):006:0> json in {key: /value/}
=> true
```
[object_class]: https://docs.ruby-lang.org/en/3.2/JSON.html#module-JSON-label-Parsing+Options
This adds additional test coverage to ShowExceptions, since one of the
possible responses it creates was not previously tested. Because of the
previous [addition][1] of Rack::Lint, this also demonstrates that the
Content-Type header needed to be fixed.
[1]: 339dda4a82356d173b62dab144870790618e40c6
Previously, when a Request had a non-authorized HTTP_HOST but an
authorized HTTP_X_FORWARDED_HOST, the HTTP_X_FORWARDED_HOST value would
be displayed as the one being blocked. However, this could be confusing
for users since that value would already be added to `config.hosts`.
This commit addresses the issue by tweaking how the blocked host is
displayed. Instead of always displaying Request#host (which will return
X_FORWARDED_HOST when present whether or not that's the host being
blocked), each host being blocked will be displayed on its own.
Co-authored-by: Daniel Schlosser <Eusebius1920@users.noreply.github.com>
The naming difference between the test harness' [file_fixture][] helper
made available through Active Support (along with the
`file_fixture_path` configuration value) and the integration test
harness' [fixture_file_upload][] is a constant source of confusion and
surprise.
Since Active Support is more ubiquitous, this commit renames the
`fixture_file_upload` method to `file_fixture_upload` to match the order
of words in `file_fixture` and `file_fixture_path`.
To preserve backwards compatibility, declare a `fixture_file_upload`
alias to be preserved into the future (or removed at a future point in
time).
[file_fixture]: https://edgeapi.rubyonrails.org/classes/ActiveSupport/Testing/FileFixtures.html#method-i-file_fixture
[fixture_file_upload]: https://edgeapi.rubyonrails.org/classes/ActionDispatch/TestProcess/FixtureFile.html#method-i-fixture_file_upload
As of Selenium 4.6, [the Selenium Manager is capable of managing Chrome
Driver installations and integrations][readme]. As of Selenium 4.11, the
Selenium Manager is capable of [capable of resolving the Chrome for
Testing installation][] path.
By omitting the `gem` declaration from the `Gemfile.tt`, newly generated
applications and applications updating their `Gemfile` in lockstep with
newer Rails versions can shed the dependency and avoid test failures
introduced by newly released Chrome versions (like, for example,
[titusfortner/webdrivers#247][]).
[readme]: 43f8ac436c (update-selenium-manager)
[titusfortner/webdrivers#247]: https://github.com/titusfortner/webdrivers/issues/247
[capable of resolving the Chrome for Testing installation]: https://github.com/rails/rails/pull/48847#issuecomment-1656756862
Co-authored-by: Titus Fortner <titusfortner@users.noreply.github.com>
This wraps test coverage for `ActionDispatch::ShowExpections` in
`Rack::Lint` middleware in order to validate that both
`ActionDispatch::ShowExceptions` and `ActionDispatch::PublicExceptions`
conform to the Rack SPEC.
It also ensures that the response headers returned by the *Exceptions
middleware respect casing (mixed case for Rack 2, lower case for Rack 3)
To ensure Rails is and remains compliant with [the Rack 3
spec](6d16306192/UPGRADE-GUIDE.md)
we can add `Rack::Lint` to the Rails middleware tests.
This adds additional test coverage to
`ActionDispatch::RemoteIp` to validate that its input and
output follow the Rack SPEC.
The only code testing this middleware are the ones for
`ActionDispatch::Request`.
Several changes were required to make the tests pass:
- `CONTENT_LENGTH` must be a string
- `SERVER_PORT` must be a string
- `HTTP_HOST` must be a string
- `rack.input` must be an IO object, with ASCII-8BIT encoding
- By leveraging `Rack::MockRequest`, we can pass the symbol :input,
and the string value, and it will be converted to an IO object
with the correct encoding.
- See [definition here](444dc8a130/lib/rack/mock_request.rb (L89-L97))
- using `Rack::MockRequest` also means that any symbol keys being passed
to setup the env, will be discarded. [Only string keys are copied.]444dc8a130/lib/rack/mock_request.rb (L156)
This adds additional test coverage to DebugExceptions to validate that
its behavior conforms to the Rack SPEC.
The only changes necessary were to use dynamic header casing for
Content-Type and Content-Length
This commit changes the router to use the expected casing for the
x-cascade header: in Rack 2, this is mixed-case, and in Rack 3, this is
lower case.
This also fixes https://github.com/rails/rails/issues/47096.
Rack 3 now allows response header values to be an Array when handling
multiple values. Newline encoded headers are no longer supported.
This commit updates `ActionDispatch::SSL#flag_cookies_as_secure!` to
be Rack-3 compliant by setting the `set-cookie` header to an Array
rather than a newline-separated String if the current Rack version is
3+.
Additionally, this commit adds `Rack::Lint` to the Rack app in the
middleware test suite so that we can ensure all of the tests are
compliant with the Rack SPEC.
In both Rack 2 and Rack 3, all headers must be strings. SERVER_PORT has
an additional requirement that it must be an Integer (represented as a
string).
When using #port= on a TestRequest, the value passed has been coerced
into an integer since it was [introduced][1]. Since this is explicitly
incorrect per both Rack 2 and Rack SPEC, the coercion is removed.
This does have the potential to change the value for users who are
checking TestRequest#headers directly, but if they are using
Request#port the value will not change because #port also coerces values
to ints.
[1]: 61960e7b37767140e9af68bd5373e06dce08492d
To ensure Rails is and remains compliant with [the Rack 3
spec](6d16306192/UPGRADE-GUIDE.md)
we can add `Rack::Lint` to the Rails middleware tests.
This adds additional test coverage to `ActionDispatch::ServerTiming` to
validate that its input and output follow the Rack SPEC.
The `Server-Timing` header definition was moved to
`ActionDispatch::Constants` and is now downcased to match the Rack 3
SPEC.
The tests that rely on a `Concurrent::CyclicBarrier` ("events are
tracked by thread") were changed since passing the required proc in the
env is not compatible with the SPEC:
```
Rack::Lint::LintError: env variable proc has non-string value
```
The same can be achieved by invoking the proc as a child Rack app.
This adds additional test coverage to RequestId to validate that its
input and output follow the Rack SPEC.
In this case, the only changes necessary were to the Request tests. This
is due to the fact that the Request and Response tests use different
classes for their Response headers. The Response tests simulate a Rails
app, where the Response headers will be a Rack::Headers object for
compatbility with both Rack 2 and 3. However, since the Request tests
are only using the Hash returned by the test app, the tests must use a
downcased header to support both Rack 2 and Rack 3.
This adds additional test coverage to HostAuthorization to validate that
its behavior conforms to Rack SPEC.
This fixes the following two issues in the reponse returned by
DebugLocks:
- Rack::Lint::Error: uppercase character in header name
Content-{Type/Length}
- Rack::Lint::Error: a header value must be a String or Array of
Strings, but the value of 'content-length' is an Integer
