Prior to this commit, the only out-of-the-box parsing that
`ActionDispatch::Testing::TestResponse#parsed_body` supported was for
`application/json` requests. This meant that `response.body ==
response.parsed_body` for HTML requests.
```ruby
get "/posts"
response.content_type # => "text/html; charset=utf-8"
response.parsed_body.class # => Nokogiri::HTML5::Document
response.parsed_body.to_html # => "<!DOCTYPE html>\n<html>\n..."
```
Using `parsed_body` for JSON requests supports `Hash#fetch`, `Hash#dig`,
and Ruby 3.2 destructuring assignment and pattern matching.
The introduction of [Nokogiri support for pattern
matching][nokogiri-pattern-matching] poses an opportunity to make assertions
about the structure of the HTML response.
On top of that, there is ongoing work to [introduce pattern matching
support in MiniTest][minitest-pattern-matching].
[nokogiri-pattern-matching]: https://github.com/sparklemotion/nokogiri/pull/2523
[minitest-pattern-matching]: https://github.com/minitest/minitest/pull/936
In the GitHub RoR monolith, we output the route URI pattern
in an HTML meta tag in our application layout for analysis
purposes. However, our current implementation is quite manual.
This change adds an attribute to requests with the URI pattern
of the matched route.
Co-authored-by: Rafael Mendonça França <rafael@rubyonrails.org>
Co-authored-by: Kate Higa <khiga8@github.com>
* Allow use of SSL-terminating reserve proxy that doesn't set headers
NGINX and other SSL-terminating reverse proxies can use HTTP headers to include forwarding information. If your stack includes SSL-termination through a network load balancer, that won't happen. You can use config.assume_ssl to address that.
* I hate these warts
* Document the new setting
* Add autoload for AssumeSSL
* Add CHANGELOG notice
Rack 3 introduces streaming bodies, which don't respond to `#each` and
MUST respond to `#call`. Ensure that the methods are correctly delegated.
`#to_ary` must also work correctly for enumerable bodies, and is used by
middleware like `Rack::ETag` to buffer enumerable bodies correctly.
Rack 3 response headers must be a mutable hash with lower-case keys. Rack
provides `Rack::Headers` as a compatibility layer for existing systems
which don't conform to this requirement. Prefer `Rack::Utils::HeaderHash`
on Rack 2, and `Rack::Headers` on Rack 3.
Remove some of the response test cases which test `nil` header keys as
these are considered invalid, and will fail with `Rack::Headers`.
In `6d5e0d2de2a8836e858962981c34aff2f76ffe3d` we added a `response=` method
that was redefining the already existed method generated by `attr_internal`.
Rack 2 includes this code, but in Rack 3 it was extracted into gems. These
gems include a v1 release compatible with Rack 2, and a v2 release
compatible with Rack v3+.
Rack 3 deprecates some of these clunky methods, and the only compatible
methods between Rack 2 and Rack 3 is to use `Rack::Response` which
includes `set_cookie` and `delete_cookie`.
When I was debugging `ActionDispatch::Request` instances in some tests, I
noticed IRB complaining that the object did not support `#inspect`, as
it was trying to print out the `method` which calls `check_method(nil)`
which fails. Don't try to validate `nil` method as it will always fail
and appears to be a valid state (when constructing an empty request as in
some tests).
The current implementation makes assumptions about the case and format of
headers. Introduce methods to handle headers in a case insensitive manner
and reduce churn when comparing with multi-value headers.
The current implementation makes assumptions about the order and case
sensitivity of cookie attributes. Introduce methods to parse those fields
and compare them semantically. Update the existing tests to take advantage
of these new assertions.
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`.
We want to use error highlight with eval'd code, specifically ERB
templates.
Previously we could only get the information we needed by setting
`keep_script_lines` to true. In Ruby 3.2 and error_highlight we added
the ability to get this information without setting `keep_script_lines`.
This change implements that new behavior for Rails.
I removed the script line changes to support this in 3.1 because it is
not in any released version.
Ruby change: https://github.com/ruby/ruby/pull/6593
Erorr highlight change: https://github.com/ruby/error_highlight/pull/26
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
In #33418, documentation from `ActionView::Helpers::RenderingHelper#render`
was copied to `ActionController::Renderer#render` with the intention of
documenting `ActionController::Rendering#render`. Since then, further
documentation has been added to `ActionController::Renderer#render`, and
`ActionController::Renderer#render` has been mistaken for
`ActionController::Rendering#render` (for example, in #46045).
This commit adds documentation to `ActionController::Rendering#render`
(which was previously `:nodoc:` because it is a simple override of
`AbstractController::Rendering#render`), and updates related
documentation to point to `ActionController::Rendering#render`.
The w3.org RFC 2616 page displays an obtrusive "This document has been
superseded" overlay. In regard to the `Cache-Control` header, RFC 2616
has been superseded by RFC 7234, which, in turn, has been superseded by
RFC 9111.
Therefore, this commit replaces links to RFC 2616 with links to either
MDN or RFC 9111.
This clarifies the `ActiveSupport::ParameterFilter` documentation, and
tweaks the example code to be more friendly to the syntax highlighter
(similar to the tweaks made for `ActionDispatch::Http::FilterParameters`
in 782bed5d450363b302e0e6aa28b7ea0aef306d9f).
This also trims the `ActionDispatch::Http::FilterParameters`
documentation, and links it to `ActiveSupport::ParameterFilter`, since
`ActiveSupport::ParameterFilter` is responsible for filter behavior.
Prior to this commit, the list of RFCs and URLs was jumbled in a single
paragraph (with no delimiters) that was associated with the `RFC2616`
constant.
This commit splits up the list, associating each RFC with its respective
constant, and incorporating each URL into a link.
This matches the indentation used in generated code, such as code from
`railties/lib/rails/generators/rails/scaffold_controller/templates/controller.rb.tt`.
This replaces two instances of /html/.match?(<string>) with the more
performant <string>.include?("html").
Performance/StringInclude was enabled in 3158bbb, however RuboCop does
not flag these two because it is unable to determine that the variable
passed to #match? is a string. In both these cases we know that the
variable must be a string (Mime::Type must be initialized with a string,
and Content-Type must be a string if present per Rack SPEC)
These were found by `rg '/\[\w ]+/\.match\?'`, and while this search
returns other entries they are either comments or in test files.
In #43487 we missed adding a changelog so that's been added here. In
addition, since this isn't a new framework default unless you are
creating a new application (and only in dev and test environments by
default) it can be easy to miss this new option. I've updated the
message to mention the option following DHH's suggestion on the original
PR.
Previously, ActionDispatch::IntegrationTest would always set
CONTENT_TYPE on the request whether or not the value being set was a
string or nil. However, Rack SPEC requires that if CONTENT_TYPE is set,
it must be a string.
Since the request_encoder can return nil for #content_type (and the
IdentityEncoder always will), IntegrationTest must check before it sets
the CONTENT_TYPE value.
A Rack::Lint test has been added to prevent regressions. Additionally,
it will make changes needed for Rack 3 more obvious when the time comes.
This patch reduces Array object allocations from some Rack middleware per each
request by reusing the Array object that wraps status, headers, and body
objects. This is a Rails version of the same improvements that has already been
pushed to Rack 3.0. https://github.com/rack/rack/pull/1887
When a host is not specified for an `ActionController::Renderer`'s env,
the host and related options will now be derived from the routes'
`default_url_options` and `ActionDispatch::Http::URL.secure_protocol`.
For example, with:
```ruby
Rails.application.default_url_options = { host: "rubyonrails.org" }
Rails.application.config.force_ssl = true
```
Before:
```ruby
ApplicationController.renderer.render inline: "<%= blog_url %>"
# => "http://example.org/blog"
```
After:
```ruby
ApplicationController.renderer.render inline: "<%= blog_url %>"
# => "https://rubyonrails.org/blog"
```
As a consequence, Action Text attachment URLs rendered in a background
job (a la Turbo Streams) will now use `Rails.application.default_url_options`.
Fixes#41795.
Fixeshotwired/turbo-rails#54.
Fixeshotwired/turbo-rails#155.
Currently if you do this:
```ruby
config.active_record.query_log_tags = [:namespaced_controller]
```
A request that's processed by the `NameSpaced::UsersController` will log as `namespaced_controller='NameSpaced%3A%3AUsersController'`.
By contrast if you set the tag to `:controller` it would log as `controller='user'`, much nicer.
This PR makes the `:namespaced_controller` formatting more similar to `:controller` - it will now log as `namespaced_controller='name_spaced/users'`.
ref: https://github.com/rails/rails/pull/46279
That PR missed the case where if you set `config.active_record.query_log_tags = [:namespaced_controller]`, it would log the controller twice:
```
/*namespaced_controller:Foo::BarController,controller:bar*
```
So this PR just fixes that bug, and tweaks the changelog entry rather than adding another one for the same bug.
This preallocates a `DEFAULT_ENV` Rack env for `Renderer` instances to
use, and avoids `dup`ing the `DEFAULTS` Hash unless the user intends to
modify it. This reduces retained allocations per controller class.
**Benchmark**
```ruby
# frozen_string_literal: true
require "benchmark/memory"
$controllers = []
Benchmark.memory do |x|
Class.new(ActionController::Base) # warmup
x.report("1 controller") do
$controllers << Class.new(ActionController::Base)
end
x.report("100 controllers") do
100.times { $controllers << Class.new(ActionController::Base) }
end
end
```
**Before**
```
Calculating -------------------------------------
1 controller 16.070k memsize ( 9.683k retained)
133.000 objects ( 40.000 retained)
50.000 strings ( 14.000 retained)
100 controllers 1.607M memsize ( 875.044k retained)
13.300k objects ( 3.308k retained)
50.000 strings ( 50.000 retained)
```
**After**
```
Calculating -------------------------------------
1 controller 15.654k memsize ( 9.347k retained)
129.000 objects ( 38.000 retained)
49.000 strings ( 14.000 retained)
100 controllers 1.565M memsize ( 841.284k retained)
12.900k objects ( 3.108k retained)
50.000 strings ( 50.000 retained)
```
This does add a `dup` to `render` because `request.routes = ...` mutates
the underlying env, which can now be `DEFAULT_ENV`. But a temporary
allocation there (likely outside of a request cycle) seems like a
reasonable trade for avoiding two retained allocations per controller
class.
Since engine initializers run later in the process, we need to run this
initializer earlier than the default.
This ensures they're all registered before the environments are loaded.
This fixes a few inaccuracies that have been present since
2db7304c2c338711b265e92a51d29121cbd702e6. For example,
`Controller.renderer` no longer returns a class, `Renderer#env` is not
defined, and changing the value of `Renderer#defaults` has no direct
effect. This also documents the user-friendly Rack env key variants.
My app was raising a `CookieOverflow` exception but it was difficult to pinpoint the cause, since error trackers and logging system generally filter out cookies.
This Pull Request has been created because I want the exception to provide additional information:
- The name of the cookie that overflowed
- The magnitude of how much it overflowed by
I am assuming that only the cookie value is sensitive, and not its name or size.
In the case where a controller subclasses an engine's controller that,
in turn, subclasses a controller that includes the application's
`url_helpers` (for example, in the "isolated engine routes and helpers
are isolated to that engine" test in `railties/test/railties/engine_test.rb`),
this commit avoids allocating a new module per controller:
```ruby
ActionController::Base.include Rails.application.routes.url_helpers
C1 = Class.new(ActiveStorage::DirectUploadsController)
C2 = Class.new(ActiveStorage::DirectUploadsController)
C1.ancestors - C2.ancestors
# BEFORE:
# => [C1, #<Module:0x...>]
# AFTER:
# => [C1]
```
This commit also modifies the `RouteHelperIntegrationTest` test to use
the controllers defined in `actionpack/test/abstract_unit.rb`.
Otherwise, `extend AbstractController::Railties::RoutesHelpers.with(...)`
happens twice -- once for `ActionController::Base` and once for
`FakeACBase` -- which causes `FooController` to include an extra module
as it flip-flops its `_routes` definition. Previously, the extra module
only defined a `_routes` method; now, the extra module would be the
memoized dup of `routes.url_helpers`, which would cause the "only
includes one module with route helpers" test to falsely fail.
Using a `request` object that only "responds to the `host`,
`optional_port`, `protocol`, and `symbolized_path_parameter` methods"
has not been possible for a long time. For example,
`symbolized_path_parameter` was renamed to `path_parameters` (without
deprecation) in 925bd975663df2e0e8613507a7c95a6945a277ac. And
`request.routes` and `request.original_script_name` methods became
required in 87a75910640b83a677099198ccb3317d9850c204. And a
`request.engine_script_name` method became required in
4080dd2f244e7c4d140f8724c2075102ea9db36e.
Therefore, this commit updates the documentation to simply require
an `ActionDispatch::Request` instance.
Prior to this commit, the format of the code example confused the syntax
highlighter. This commit formats each explanation as a code comment,
and tweaks their wording for clarity.
These references were missed when `ActionController::UrlFor` was
converted to `ActionDispatch::Routing::UrlFor` in
226dfc2681c98deaf14e4ae82e973d1d5caedd68.
- rename testcases
- reduce redundant parameters
- rename tested action methods
- move skip_parameter_encoding near by definiton of action
- add test for param_encoding
- Correct controller path "app/controller/books_controller.rb" to "app/controllers/books_controller.rb"
- Add space after "NOTE:"
Ref - https://github.com/rails/rails/pull/46342
Adding documentation to the API doc for `ActionDispatch::Routing::Mapper::Resources#draw` method,
inspired by the Rails Guide routing section. Also removed extra colon in title.
This commit adds `ActionView.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `actionview/lib` with
`ActionView.deprecator`. This commit also replaces a call to Ruby's
`Module#deprecate_constant` with Rails' `DeprecatedConstantProxy`, so
that its deprecation behavior can be configured using
`ActionView.deprecator`.
Additionally, this commit adds `ActionView.deprecator` to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
This commit also removes a few defunct `assert_deprecated` calls that
were not failing because they were nested in `assert_raises`, and the
raised error prevented checking the deprecation. (One was mistakenly
kept in d52d7739468153bd6cb7c629f60bd5cd7ebea3eb when converting
`test_render_file_with_errors` to `test_render_template_with_errors`;
the other two were added in dd9991bac598bb5da312278a749cf85e19b027cc but
not removed when the deprecation was completed in
85ecf6e4098601222b604f7c1cbdcb4e49a6d1f0.)
This commit adds `ActionDispatch.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `actionpack/lib/action_dispatch`
with `ActionDispatch.deprecator`.
Additionally, this commit adds `ActionDispatch.deprecator` to
`Rails.application.deprecators` so that it can be configured via
settings such as `config.active_support.report_deprecations`.
This commit adds `AbstractController.deprecator` and
`ActionController.deprecator`, and replaces all usages of
`ActiveSupport::Deprecation.warn` in `actionpack/lib/action_controller`
with the latter.
Additionally, this commit adds `ActionController.deprecator` to
`Rails.application.deprecators`. Because `AbstractController` does not
have its own railtie to do the same, `AbstractController` and
`ActionController` use the same deprecator instance. Thus, both can be
configured via `Rails.application.deprecators[:action_controller]` or
via config settings such as `config.active_support.report_deprecations`.
Some goals here:
- Be less obtuse with word choices for folks who use English
as a second language.
- Stop referring to view templates as "templates" when the
literal directory is called `app/views` which has and will
confuse beginners (otherwise let's rename the directory).
- Stop formatting the HTML with `<br>` tags like it's 2005
and let the text flow with the viewport as appropriate.
- Give a *concrete* example of the naming relationship between
a controller and its view template, based on
[the ActionView Rails Guides][1].
I also tried to clarify the frankly confusing copy explaining
the 204 No Content rendering case, which I think should
probably mention that such endpoints shouldn't be allowed to
render HTML to avoid this error, but that might be pushing
the scope of the error feedback a bit too far.
[1]: https://guides.rubyonrails.org/layouts_and_rendering.html
The Rack specification[1] states that header values
must be a `String` instance (or array thereof).
When setting up the content-type header based on the
file extension, `send_stream` assigns the whole
`Mime::Type` object, instead of its `String`
representation.
This change ensures that all three ways of setting
the content-type header (implicit via extension,
implicit via a symbol, and explicit via a string)
all result in a `String` instance being assigned as
the header's value.
[1] 0a62f75eee/SPEC.rdoc (label-The+Headers)
Fixes https://github.com/rails/rails/issues/46103
An issue exists if you set `config.active_record.query_log_tags` to an array that includes `:controller`, `:action`, or `:job`; the relevant item will get duplicated in the log line. This occured because the relevant railties would add the item to `config.active_record.query_log_tags` again during setup. This PR fixes that by only adding those items to the config if they aren't already set.
The issue proposed more documentation to work around this, but I think it's a bug and should be fixed directly.
This is useful when you want to filter your own hashes based on the
same parameter filter as the request.
Examples of this are exception notification and logging, that needs to
load headers or even custom values from the request, and want to keep
filtering those.
Screenshot filenames are derived from test names which can contain
special characters. These special characters may not be supported by
CI systems like Github Actions. Replacing all non word characters
ensures compatibility.
We recently let a few very easy to avoid warnings get merged.
The root cause is that locally the test suite doesn't run in
verbose mode unless you explictly pass `-w`.
On CI warnings are enabled, but there is no reason to look at the
build output unless something is failing. And even if one wanted
to do that, that would be particularly work intensive since warnings
may be specific to a Ruby version etc.
Because of this I believe we should:
- Always run the test suite with warnings enabled.
- Raise an error if a warning is unexpected.
We've been using this pattern for a long time at Shopify both in private
and public repositories.
We have access to the path from the backtrace location object. If we
use the path of the ERB as the key, then anytime the ERB changes it'll
just overwrite that template instance in the error handling hash
This commit maps the column information returned from ErrorHighlight in
to column information within the source ERB template. ErrorHighlight
only understands the compiled Ruby code, so this commit adds a small
translation layer that converts the values from ErrorHighlight in to the
right values for the ERB source template
We should get out of the business of parsing backtraces and only use
backtrace locations. Backtrace locations have the file and line number
information baked in, so we don't need to parse things anymore
This commit adds a SyntaxErrorProxy object to active support and wraps
syntax error exceptions with that proxy object. We want to enhance
syntax errors with information about the source location where they
actually happened (normally the backtrace doesn't contain such info).
Rather than mutating the original exception's backtrace, this wraps it
with a proxy object.
Eventually we will implement backtrace_locations on the proxy object so
that the exception handling middleware can be updated to _only_ deal
with backtrace_locations and never deal with raw `backtrace`
We are currently mutating exception objects and I would like to stop
doing that. Unfortunately the views are calling many methods directly
on the exception and expecting that the mutations exist.
This patch refactors the templates so that they ask the ExceptionWrapper
class for information about the exception rather than directly asking
the exception object itself
`selenium-webdriver` v4.5.0 adds more entries ("acceptInsecureCerts" and
"moz:debuggerAddress") to the `as_json` output for
`Selenium::WebDriver::Firefox::Options`, causing an exact comparison of
the Hash to fail.
See SeleniumHQ/selenium@58f5833ba0.
Gotta be honest, this is so I can make some hacks. Basically I would
like an engine to specify where to find rescue templates, and currently
there's no way to add search paths to the debug view lookup context.
This commit turns the template path in to an array (that I plan to
mutate, but nobody should do that besides me until we make an actual
good API).
I added the `dup` in `initialize` so in case the array is accidentally
mutated we don't leak memory.
Previously, the method always asserts the status is `:redirect` which
allows for any kind of 3XX response. However, sometimes it is worthwhile
to precise the status code of the redirect. For example, a Rails
application may want to verify the redirect is a 301 (Moved Permanently)
and not the default 302 (Found). The new method argument makes this
convenient to do in one assertion.
Rubinius has not been maintained since May 2020 and based on the
discussion at https://github.com/rails/rails/pull/44984 ,
I think we can remove Rubinius specific code from Rails.
Previously ActionDispatch::ServerTiming would subscribe and unsubscribe
on each request. This could cause issues with the internal stacks of
ActiveSupport::Notifications, particlularly under the previous AS::N
implementation which used thread-local stacks for every subscriber
(the new implementation has mostly mitigated this).
Additionally, the previous ServerTiming implementation did not report
metrics correctly in a multi-threaded environment.
This commit works around both of these issues by using a single global
subscription, which collects events into a per-thread Array.
Without this change if action_dispatch.cookies_serializer is set to
json and the app tries to read a marshal-serialized cookie, it will
raise a JSON::ParserError which won't clear the cookie and force app
users to manually clear the cookie in their browser.
(See #45127 for original bug discussion)
This change incorporates to Rails a feature called error_highlight that
has been available since Ruby 3.1. This allow Rails' error report screen
to display the fine-grained location where an error occurred (not only a
line number but also beginning and end column numbers of the code
fragment).
For ErrorHighlight, see https://bugs.ruby-lang.org/issues/17930 in
detail.
To use error_highlight, ExceptionWrapper now prefers
`Exception#backtrace_locations` (since Ruby 2.1), which returns an array
of `Thread::Backtrace::Location`s, instead of `Exception#backtrace`.
This is because error_highlight requires `Thread::Backtrace::Location`
to locate the column where an error was raised.
Co-Authored-By: John Hawthorn <john@hawthorn.email>
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
The various LogSubscriber subclasses tend to subscribe to events
but then end up doing nothing if the log level is high enough.
But even if we end up not logging, we have to go through the
entire notification path, record timing etc.
By allowing subscribers to dynamically bail out early, we can
save a lot of work if all subscribers are silenced.
Right now many helpers have to deal with two modes of operation to
capture view output.
The main one is to swap the `@output_buffer` variable with a new buffer.
But since some view implementations such as `builder` keep a reference
on the buffer they were initialized with, this doesn't always work.
So additionally, the various capturing helpers also record the buffer
length prior to executing the block, and then `slice!` the buffer back
to its original size.
This is wasteful and make the code rather unclear.
Now that `OutputBuffer` is a delegator, I'd like to refactor all this
so that:
- @output_buffer is no longer re-assigned
- A single OutputBuffer instance is used for the entire response rendering
- Instead capturing is done through `OutputBuffer#capture`
Once the above is achieved, it should allow us to enabled Erubi's
`:chain_appends` option and get some reduced template size and some
performance.
Not re-assigning `@output_buffer` will also allow template to access
the local variable instead of an instance variable, which is cheaper.
But more importantly, that should make the code easier to understand
and easier to be compatible with `StreamingBuffer`.
Routes take a long time to draw. Over time, a Rails app can become slow
to boot simply because of how many routes it has. This script can be
used to detect routes that are drawn, but aren't actually valid.
Removing routes this script detects can help speed up your app and
remove dead code.
Example:
```
> bin/rails routes --unused
Found 2 unused routes:
Prefix Verb URI Pattern Controller#Action
one GET /one(.:format) action#one
two GET /two(.:format) action#two
```
Follow-up to #45501.
The Rack base class that `CookieStore` inherits from [always sets
`:same_site`][1]. Thus, `options.key?(:same_site)` always returns true
for session cookies, preventing a default value from being set.
It would be possible to change Rack to conditionally set `:same_site`,
but, from Rack's perspective, it has no reason to not set `:same_site`,
because it treats a `nil` value the same as no value.
Therefore, this commit specifies a default `:same_site` in `CookieStore`,
which simply defers to `request.cookies_same_site_protection` as
`CookieJar` does.
Fixes#45681.
[1]: https://github.com/rack/rack/blob/2.2.4/lib/rack/session/abstract/id.rb#L398-L402
This commit just ensures we're green with the main branch of rack test.
The changes are things we should have done anyway, and are backwards
compatible with older versions of rack test
Since 7ccaa125ba it's not been possible to not include `SameSite` on your cookies. `SameSite` is recommended, but it's not a required field, and you should be able to opt out of it.
This PR introduces that ability: you can opt out of `SameSite` by passing `same_site: false`.
```ruby
cookies[:foo] = { value: "bar", same_site: false }
```
Previously, this incorrectly set the `SameSite` attribute to the value of the `cookies_same_site_protection` setting. https://github.com/rails/rails/pull/44934 added docs saying that you could pass `nil` as a value, but that also would fall back to the default (`:lax`).
Fixes https://github.com/rails/rails/issues/45489
- Adds `anchor: true` to the Action Cable server mount, so that it only strictly matches `/cable` rather than anything that starts with that.
- Uses `reverse_merge` instead of `merge` in `Mapper#mount`, so that you can override these options if you need to.
In minitest/minitest@6e06ac9 minitest changed such that it now accepts
`kwargs` instead of requiring kwargs to be shoved into the args array.
This is a good change but required some updates to our test code to get
the new version of minitest passing.
Changes are as follows:
1) Lock minitest to 5.15 for Ruby 2.7. We don't love this change but
it's pretty difficult to get 2.7 and 3.0 to play nicely together with
the new kwargs changes. Dropping 2.7 support isn't an option right
now for Rails. This is safe because all of the code changes here are
internal methods to Rails like assert_called_with. Applications
shouldn't be consuming them as they are no-doc'd.
2) Update the `assert_called_with` method to take any kwargs but also
the returns kwarg.
3) Update callers of `assert_called_with` to move the kwargs outside the
args array.
4) Update the message from marshaled exceptions. In 5.16 the exception
message is "result not reported" instead of "Wrapped undumpable
exception".
Co-authored-by: Matthew Draper <matthew@trebex.net>
activerecord-session_store was removed in 0ffe190, and has been
displaying a special error message when missing since Rails 4.0.
Replace the specific error message so that third party stores get nicer
error handling as well
This documentation was correct when it was written in 6e75455, however
`headers` has moved a few times since:
- added to ActionController::Http in 216309c as part of the new_base
- Http was renamed to Metal in 52798fd
- headers was changed from an independent hash to a delegation in
51c7ac1 and 54becd1
Added docs for Metal#request, Metal#response, and Metal#headers that can
be linked to from Response. The recommendation to use Metal delegation
methods instead of methods on Response was also removed due to a number of
docs/guides demonstrating the opposite.
In
f075be3dcb
did_you_mean and error_highlight now use `detailed_message` over
`message` to display errors.
For cases where we are testing `message`, in 3.2 and above we need to
test against `detailed_message` instead.
As far as I can tell in a Rails console when these errors are raised the
`detailed_message` is used so we shouldn't need to make other changes to
Rails. The only case where this isn't true is in the Railties changes -
we are explicitly formatting the did you mean message so we need to be
sure to call `detailed_message` here.
This fixes most of the failing tests for ruby-trunk.
Previously, as of 80aaa111884247e6aa17b7bbab268c7719847521,
ActionController::Parameters has defined hash as:
[@parameters.hash, @permitted].hash
Defining hash means that eql? must be defined, and eql? must be at
least as strict as the hash value generated. That is, for any two
objects which return a different hash value, `a.eql?(b)` should return
false. Otherwise, because hash values have a random seed added, and in
some cases have only some of their bits compared, their behaviour in a
hash becomes undefined. Previously we were breaking this expectation by
allowing a deprecated comparison between Parameters and a plain hash.
This commit fixes eql? to match hash, only returning true when the class
matches as well as the permitted? and parameters values (ie. eql? never
allows the deprecated relaxed equality branch).
This also adds the class to the hash and eql? check, which previously
wasn't there, which isn't strictly necessary to fix this but I think is
a best practice.
Since 5745a3c0928ee5604ce80af19348efb42189f1d6, if a controller defines
a `headers` method it will be called by this line, and the return value
will be mutated. This was also preventing the "Vary" header from being
sent to the client.
Co-authored-by: Oleksandr Bezruchenko <alex.bezruchenko@intercom.io>
Co-authored-by: Iliana Hadzhiatanasova <iliana.hadzhiatanasova@intercom.io>
Kernel#method was redefined so one couldn't do for instance.
method(:POST).source_location
Now when called without arguments it returns the method of the
request and when called with arguments it uses Kernel#method
Which makes debugging easier
Co-authored-by: Joé Dupuis <joe@dupuis.io>
Fixes https://github.com/rails/rails/issues/45034
Currently helpers that are generated using `helper_method` cannot be used in `content_security_policy` and `permissions_policy`, this is because the use of `yield` causes `self` to be set incorrectly. By using `instance_exec` we ensure the scoping is correct so that you can access the same methods you'd be able to if you wrote your own `before_action`.
Currently if you use `helper_method` to define a method, and inside that method you get an error, the backtrace is off by one line.
This PR fixes that so that the backtrace now points to the line where you called `helper_method`.
It is best practice to include the unexpected value in logs
and error message as it can often make it much easier to understand
where the error come from.
Fix: https://github.com/rails/rails/issues/44923
The fix may seem very ad hoc, but this methods assumes all params
come from Rack, hence are mutable. So checking for frozen is a decent
proxy for ignoring the router defaults.
Add the default status code returned by `ActionDispatch::Routing::Redirection#redirect` in the RDoc comment.
Include an example
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
As the `reset` method is called without further conditionals in line 342, all
custom strategies must implement this method as well.
This may have been an oversight in one of the many iterations of #44283.
It was added in 66f8997 to be compatible with Rack::ContentLength.
However, to_ary was removed from Rack::Response in 2.1.0, and
Rack::ContentLength stopped checking for response bodies to define
to_ary in 2.2.0. In addition, Rack 3 will eventually require response
bodies that define to_ary to have a proper return value.
Since the minimum supported Rack version is already 2.2.0, to_ary can
be safely removed now.
