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 (3ea2857943) 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]: 48cb8b3e70
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]: 339dda4a82
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]: 61960e7b37
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
This adds additional test coverage to Static to validate that its
behavior conforms to the Rack SPEC.
The test changes are just downcasing headers where appropriate:
- the Static `headers` params is purely user configured headers, so its
reasonable to expect these shoud be correct for an application's Rack
version
- header assertions can use downcased headers because Rack::MockRequest
returns a Rack::Response, which uses Rack::Headers internally (so
either casing will work)
Additionally, the unconditionally downcased headers in the Static
middleware were updated to be conditional based on the Rack version to
ensure that this middleware remains fully compatible with other Rack 2
middleware.
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::MiddlewareStack` to validate that its input and output follow the Rack SPEC.
In this case, no changes are required, and the additional test
will ensure this middleware remains compliant with the Rack SPEC.
This commit refactors the StaticTests class in preparation for adding
Rack::Lint to the tests.
The first change is inlining the StaticTests module into the StaticTest
class. It was originally extracted into a module when Static was
[changed][1] to support passing multiple root paths, but support for
multiple paths has since been [removed][2].
The second change is to move all Rack App creation into a single method.
This will make it extremely easy to add Rack::Lint to the App in a
followup commit.
[1]: 401cd97923
[2]: d5ad92ced1
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::Executor` to
validate that its input and output follow the Rack SPEC.
This also removes some tests that were asserting the body object
passed to `ActionDispatch::Executor` and not the Rack SPEC.
See also https://github.com/rack/rack/issues/2100.
This adds an additional test to the ActionDispatch::Cookies middleware
test suite to ensure that the middleware sets the expected cookie header
when the request contains a cookie jar. Additionally, the test wraps the
Cookies middleware in Rack::Lint to ensure that ActionDispatch::Cookies
complies with the Rack SPEC.
This adds additional test coverage to PermissionsPolicy::Middleware to
validate that it conforms to the Rack SPEC.
The only changes necessary were to use the appropriate header casing for
Content-Type and Feature-Policy. Since this was the only usage of the
CONTENT_TYPE constant, I opted to remove it, but I can replace it with a
DeprecatedConstantProxy if that's more desirable.
This adds additional test coverage to ActionableExceptions to validate
that its behavior conforms to the Rack SPEC.
The changes neccesary were to ensure that Response headers are downcased
when using Rack 3. For Content-Type and Content-Length, this is trivial
because Rack provides constants who's casing is dependent on the version
(Rack 2 is mixed, and Rack 3 is downcased). Since Rack does not include
a LOCATION constant, the Response::LOCATION constant was updated to
have a downcased value when using Rack 3.
Additionally, there was some missing coverage for invalid redirect URLs
which was addressed as well.
This adds additional test coverage to HostAuthorization to validate that
its behavior conforms to the Rack SPEC.
By using Rack:: constants for Content-Type and Content-Length, we are
able to use the "correct" versions of the headers for applications using
each Rack version.
Additionally, two tests had to be updated that use an ipv6 address
without brackets in the HOST header because Rack::Lint warned that these
addresses were not valid HOST values. Rack::Lint checks HOST headers using
`URI.parse("http://#{HOST}/")`, and from what I could find, this
requirement follows RFC 3986 Section 3.2.2:
```
host = IP-literal / IPv4address / reg-name
IP-literal = "[" ( IPv6address / IPvFuture ) "]"
IPvFuture = "v" 1*HEXDIG "." 1*( unreserved / sub-delims / ":" )
```
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.
There was no test file for ActionDispatch::AssumeSSL, so this change
adds one and validating that its input and output follow the Rack SPEC.
This adds additional test coverage for ActionDispatch::Callbacks by
validating that its input and output follow the Rack SPEC.
The `"rack.input" => StringIO.new("")` header value raised the following error:
```
Rack::Lint::LintError: rack.input #<StringIO:0x00007fd7513fe550> does not have ASCII-8BIT as its external encoding
```
Since this header is not required for the test, it is now removed.
When development tools try to load Rails components, they sometimes end up loading files that will error out since a dependency is missing. In these cases, the tooling can catch the error and change its behaviour.
However, since the warning is printed directly to `$stderr`, the tooling cannot catch and suppress it easily, which ends up causing noise in the output of the tool.
This change makes Rails print these warnings using `Kernel#warn` instead, which can be suppressed by the tooling.
The set of legal characters for an HTTP header value is described
in https://datatracker.ietf.org/doc/html/rfc7230\#section-3.2.6.
This commit adds a check to redirect_to that ensures the
provided URL does not contain any of the illegal characters.
Downstream consumers of the resulting Location response header
may accidentally remove the header if it does not comply with the RFC
resulting in unexpected behavior.
Related to [CVE-2023-28362].
This middleware has been logging at a FATAL level since the first
[commit][1] in Rails (the code originally lived in
actionpack/lib/action_controller/rescue.rb). However, FATAL is
documented in the Ruby Logger [docs][2] as being for "An unhandleable
error that results in a program crash.", which does not really apply to
this case since DebugExceptions is handling the error. A more
appropriate level would be ERROR, which the Ruby Logger docs describe as
"A handleable error condition."
This commit introduces a new configuration for the DebugExceptions log
level so that new apps will have it set to ERROR by default and ERROR
can eventually be made the default.
[1]: db045dbbf6
[2]: https://ruby-doc.org/3.2.1/stdlibs/logger/Logger/Severity.html
For local environments (def and test), we create a secret file. However this file is called development_secret.txt, which imho is confusing as it is used by both dev and test environments.
This commit renames the file and related code to local_secret.
* Unlink the Rails module automatically
* Inline the documentation links for unicorn and passenger
* Use RDoc fixed-width for passenger_buffer_response instead of markdown
* TIL: about linking to headings, so fixed that for "Middlewares" section
RackBody is the final body object returned by the Rack app
(`Rails.application`). This test that it conforms to the spec
instead of testing on the underlying response.
ActionDispatch::Response delegates #to_ary to the internal ActionDispatch::Response::Buffer,
defining #to_ary is an indicator that the response body can be buffered and/or cached by
Rack middlewares, this is not the case for Live responses so we undefine it for this Buffer subclass.
Puma raises an exception trying to call #to_ary in Live::Buffer
expecting it to return an array if defined:
188f5da192/lib/puma/request.rb (L183-L186)
The rack spec requires the header object to be an unfrozen hash.
c8e9822183/SPEC.rdoc?plain=1#L240
Rack::ETag was buffering and making a copy of the response,
so the freeze was not effective anyway.
Plus we are freezing the hash too early, preventing middlewares
from modifying it. It causes crash with gems like rack-livereload.
I started having crashes on some pages (like the internal
http://localhost:3000/rails/info/routes) because of rack-livereload
hitting the frozen hash after the rack 3 upgrade.
Also we're not consistent with the protection. We're not preventing
users from adding cookies. The cookie jar is already flushed,
therefore it doesn't try to change the headers and never triggers the
frozen hash error.
Previously, `ActionDispatch::Static` would always merge a "content-type"
header into the headers returned from `Rack::Files`. However, this would
potentially lead to both a "Content-Type" header and a "content-type"
header when using Rack 2.
This commit fixes the issue by using `Rack::CONTENT_TYPE` to determine
which version of the header to set in `ActionDispatch::Static`. In both
versions of Rack it will use the same version of the header as
`Rack::Files`.
The tests added have to use `@app.call` instead of
`get()`/`Rack::MockRequest` because `Rack::Response` actually does the
correct thing already by using `Rack::Util::HeaderHash` so it covers up
the issue in tests.
Mario Caropreso