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 (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.
Turbo frames on turbo-rails 1.4 (current default in Rails 7) don't
break out of the frame to load the error response from the DebugView
middleware like they used to. It requires the turbo-visit-control meta set to reload or it
fails silently.
Accept headers allow parameters to be passed. They can contain quotes
that need to be handled differently. These quoted strings can contain
commas, which are not considered as delimiters of accept headers.
Additionally, all parameters before the q-parameter should be used to
lookup the media-type as well. If no media-type with the parameters is
found, a fallback is introduced to the media-type without any parameters
to keep the same functionality as before.
Fix#48052
The `cookies` method was not defined on ActionController::Base making the
permalink to the method not work.
Changing it to ActionController::Cookies make the reference a link.
The url_for helper now supports a new option called `bind_params`.
This is very useful in situations where you only want to add a required
param that is part of the route's URL but for other route not append an
extraneous query param.
Given the following router...
```ruby
Rails.application.routes.draw do
scope ":account_id" do
get 'dashboard' => 'pages#dashboard', as: :dashboard
get 'search/:term' => 'search#search', as: :search
end
delete 'signout' => 'sessions#destroy', as: :signout
end
```
And given the following `ApplicationController`
```ruby
class ApplicationController < ActionController::Base
def default_url_options
{ bind_params: { account_id: "foo" } }
end
end
```
The standard URLHelpers will now behave as follows:
```ruby
dashboard_path # => /foo/dashboard
dashboard_path(account_id: "bar") # => /bar/dashboard
signout_path # => /signout
signout_path(account_id: "bar") # => /signout?account_id=bar
search_path("quin") # => /foo/search/quin
```
UrlRewriter has been deleted in 2010 e68bfaf1fe1a7890a67af6f444281185f507cf9e
The url_rewriter_test is really testing url_for. Most of the tests are
identical.
This also move a couple tests that were not present in
`url_for_test.rb`.
Before this commit, some calls to render were hard-coding error
highlight as "not available". This was causing some error pages to show
the "you should install error highlight" message even though the right
version of error highlight was installed.
This commit adds a delegate method to the DebugView class so that the
debugging related templates can just ask whether or not error highlight
is available via a method call. That way we don't need to rely on
passing locals everywhere. The down side is that this change requires
all "rescue" templates to be rendered within the context of a DebugView
class (but I think that's OK)
When the Authorization header would contain a set of delimited values
where one or more values were blank, an ArgumentError would be raised.
This resolves that by removing blank values during parsing of the
Authorization header.
Also add some additional words to make it clear that the modules also
implement handling the exceptions configured with rescue_from, because
it was not immediately clear that happened without reading the code.
Most of these are redundant because rdoc handles these itself, but
`titlecase` on `ActiveSupport::Inflector` does not exist so that one is
just incorrect.
- Small wording tweaks for grammar or consistency
- Add links to methods/classes when possible, and fix some cases where
there were links but shouldn't be (`API`, `Testing`, etc.)
- Fixed `call-seq` for `each_key`
- Change `has_key?`, `key?`, and `member?` to aliases instead of
delegates so that they are documented as aliases (This is how the
methods are documented for Hash in Ruby)
- Remove explicit "also aliased as" docs because rdoc does this already
- Add `:nodoc:` to `EMPTY_ARRAY` and `EMPTY_HASH` constants since these
are internal optimizations
Background
----------
During integration tests, it is desirable for the application to respond
as closely as possible to the way it would in production. This improves
confidence that the application behavior acts as it should.
In Rails tests, one major mismatch between the test and production
environments is that exceptions raised during an HTTP request (e.g.
`ActiveRecord::RecordNotFound`) are re-raised within the test rather
than rescued and then converted to a 404 response.
Setting `config.action_dispatch.show_exceptions` to `true` will make the
test environment act like production, however, when an unexpected
internal server error occurs, the test will be left with a opaque 500
response rather than presenting a useful stack trace. This makes
debugging more difficult.
This leaves the developer with choosing between higher quality
integration tests or an improved debugging experience on a failure.
I propose that we can achieve both.
Solution
--------
Change the configuration option `config.action_dispatch.show_exceptions`
from a boolean to one of 3 values: `:all`, `:rescuable`, `:none`. The
values `:all` and `:none` behaves the same as the previous `true` and
`false` respectively. What was previously `true` (now `:all`) continues
to be the default for non-test environments.
The new `:rescuable` value is the new default for the test environment.
It will show exceptions in the response only for rescuable exceptions as
defined by `ActionDispatch::ExceptionWrapper.rescue_responses`. In the
event of an unexpected internal server error, the exception that caused
the error will still be raised within the test so as to provide a useful
stack trace and a good debugging experience.
This commit adds documentation to ShowExceptions explaining how it
should be configured in Rails applications. In addition, it adds more
`<code>` blocks to fix the formatting of some code snippets and prevent
the page from linking to itself.
This test was introduced in #19904.
In #21368 a bunch of test setup was removed, but the assignment
of `@set` was duplicated.
Removing the extraneous test setup means the test is identical to the
`test_cart_inspect` test.
This removes the test entirely.
This commit adds support for `:message_pack` and `:message_pack_allow_marshal`
as serializers for `config.action_dispatch.cookies_serializer`, just
like `config.active_support.message_serializer`.
The `:message_pack` serializer can fall back to deserializing with
`AS::JSON`, and the `:message_pack_allow_marshal` serializer can fall
back to deserializing with `AS::JSON` or `Marshal`. Additionally, the
`:marshal`, `:json`, and `:hybrid` / `:json_allow_marshal` serializers
can now fall back to deserializing with `AS::MessagePack`. These
behaviors make it easier to migrate between cookies serializers.
Fix: https://github.com/rails/rails/issues/48156
The assumption here is that in the overwhelming majority of
cases, all formats are valid.
So we first check if any of the formats is invalid before duping
the details hash and filtering them.
Additonally, by exposing a (non-public) `valid_symbols?` method, we
can check symbols are valid without resporting to `Array#%` which
would needlessly duplicate the `formats` array.
The `Type` class was introduced in https://github.com/rails/rails/pull/23085
for the sole purpose of breaking the dependency of Action View on Action Dispatch.
Unless you are somehow running Action View standalone, this is actually
never used.
So instead of delegating, we can use constant swapping, this saves us
a useless layer.
Ultimately we could consider moving `Mime::Types` into Active Support
but it requires some more thoughts.
Since 3.0, Rack doesn't guarantee rewindable request body streams.
Therefore Rack doesn't rewind the body after parsing the POST params
like it use to.
Since this is a test request, we can guarantee the stream is rewindable
and do it in the test.
Rails has incorrectly been adding leading dots to cookie domain values
when the `domain: :all` option is present.
This leading dot was required in cookies based on [RFC 2965][rfc2965]
(October 2000), but [RFC 6265][rfc6265] (April 2011) changed that
behaviour, making a leading dot strictly incorrect. Todays browsers aim
to confirm to RFC6265 with repect to cookies.
The new behaviour is that *any* cookie with an explicitly passed domain
is sent to all matching subdomains[[ref][mdn]]. For a server to indicate
that only the exact origin server should receive the cookie, it should
instead pass *no* domain attribute.
Despite the change in behaviour, browser devtools often display a cookie
domain with a leading dot to indicate that it is valid for subdomains -
this prefixed domain is *not* necessarily the raw value that was passed
in the Set-Cookie header. This explains why it's a common belief among
developers that the leading dot is required.
RFC6265 standard gives UAs an algorithm to handle old-style cookie
domain parameters (they can drop a leading dot if present), so it's
unlikely that this error would ever have had any effect on web browsers.
However, cookies generated this way can't be processed by Ruby's own
CGI::Cookie class:
> CGI::Cookie.new "domain" => ".foo.bar", "name" => "foo"
ArgumentError: invalid domain: ".foo.bar"
Newer versions of the Ruby CGI library accomodate the same fallback
behaviour (dropping the extra dot) but this isn't a justification for it
being the right way to set a cookie.
[mdn]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#domain_attribute
[rfc2965]: https://www.rfc-editor.org/rfc/rfc2965#section-3.2
[rfc6265]: https://www.rfc-editor.org/rfc/rfc6265#section-4.1.1
In larger route files, or when routes are spread across multiple files,
it can be difficult to get from the output of the route inspector to the
relevant route definition.
This commit adds a route source location to the route, and uses that in
the HtmlTableFormatter (for rails/info and the debug exceptions
middleware) and the Expanded formatter (for `rails routes -E`).
To avoid doing extra work in production, it only sets the source location
in development.
This commit injects the application's backtrace cleaner so we can use it
to remove the rails root from the path. This also means we don't get
source locations for the routes defined by Rails.
If mounting an engine from a gem, we'll get a source location for where
we mount it in the application, but not for the routes defined in the
gem itself. That's probably good enough, since Rails already prints
routes for an engine separately under the title "Routes for
Foo::Engine".
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Co-authored-by: Luan Vieira <luanzeba@github.com>
Co-authored-by: Daniel Colson <composerinteralia@github.com>
In 7c94708d24a4185075f656626ce4b14c9604ffd3 the READMEs were included for
the main framework pages of the API documentation, except for Action Pack.
As Action Pack doesn't define any code in the ActionPack namespace, only
it's included modules (Action Dispatch, Action Controller and Abstract
Controller) are documented.
This adds documentation intro's to the main page for Action Controller
and Action Dispatch. The content was copied from the Action Pack README.
As Abstract Controller isn't mentioned there, it is skipped for now.
[ci-skip]
This is the same optimization applied in
a63ae913df
which I proposed in https://github.com/rails/rails/pull/47714
Here's the benchmark:
require "bundler/inline"
ROOT_STRING = '/'
TEST_PATH = "/some/path"
gemfile(true) do
source "https://rubygems.org"
gem "benchmark-ips"
end
Benchmark.ips do |x|
x.report("path[0]") do
TEST_PATH[0] != ROOT_STRING
end
x.report("path.start_with?") do
TEST_PATH.start_with?(ROOT_STRING)
end
x.compare!
end
Warming up --------------------------------------
path[0] 942.044k i/100ms
path.start_with? 1.556M i/100ms
Calculating -------------------------------------
path[0] 9.463M (± 0.9%) i/s - 48.044M in 5.077358s
path.start_with? 15.611M (± 0.2%) i/s - 79.352M in 5.083056s
Comparison:
path.start_with?: 15611192.8 i/s
path[0]: 9463245.0 i/s - 1.65x slower
The sole purpose of `ActionController::Rendering#_normalize_args` is to
store the given block in `options[:update]`. This behavior was added
long ago in 6923b392b740f2346326634532b40cf24a0f26ef (as [part of
`ActionController::Base#_normalize_options`][part-of]) to support RJS.
Rails no longer supports RJS, so this override is no longer necessary.
[part-of]: 6923b392b7 (diff-febf2f89e7c197d6a9a7077c96031c68b2b7ac4d8ce7ec634de92b164e5f69adR100)
Expands the search field on the rails/info/routes page to also search:
* Route name (with or without a _path and _url extension)
* HTTP Verb (eg. GET/POST/PUT etc.)
* Controller#Action
because it's not obvious that the search field is currently only
restricted to the route paths.
Prevents horizontal scrolling on the rails/info/routes page when there
are long route names by introducing styled line breaks so that the table
will fit within the rendered width of the browser.
This is particularly relevant when there are a lot of nested namespaces
in a rails project and makes the page more readable, especially when
filtering with a search query.
The table headings have also been left-aligned so that they line up more
intuitively with the content and now that the table is no longer
horizontally scrolling, less space has been explicitly allocated for the
HTTP Verb column.
This is a refactor of the `Registry` module added in https://github.com/rails/rails/pull/47347. This is an attempt to
minimize the namespace conflcits that will happen when users will have a top level `Registry` module which can cause
incorrect behavior
Replace ActionView::ViewPaths::Registry with ActionView::PathRegistry
* 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>
`ActionDispatch::Static` uses mixed-case headers and merges them with
lower case headers. This produces duplicate headers. Prefer lowercase
headers to avoid this situation.
Since rails/rails#47296, nothing sets the fullpath early, so changing
the path of a request, and then calling original_fullpath returns the
updated fullpath. This is a controller testing specific bug as
integration tests and real requests always have this header set, so I
think controller tests should too.
## Summary
This PR bumps RuboCop Performance to 1.16.0 and suppresses the following new offenses:
```console
% bundle exec rubocop
(snip)
Offenses:
actionpack/lib/action_dispatch/routing/mapper.rb:309:16:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /#/.match?(to)
^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/routing/mapper.rb:1643:18:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /#/.match?(to)
^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/routing/route_set.rb:887:67:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
path = Journey::Router::Utils.normalize_path(path) unless %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/assertions/routing.rb:86:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(expected_path)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/assertions/routing.rb:205:14:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actionpack/lib/action_dispatch/testing/integration.rb:235:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if %r{://}.match?(path)
^^^^^^^^^^^^^^^^^^^^
actiontext/bin/webpack:18:6:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 150))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
actiontext/bin/webpack-dev-server:18:6:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 150))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb:120:64:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
elsif column.type == :uuid && value.is_a?(String) && /\(\)/.match?(value)
^^^^^^^^^^^^^^^^^^^^
railties/lib/rails/commands/secrets/secrets_command.rb:28:12:
C: [Correctable] Performance/StringInclude: Use String#include? instead of a regex match with literal-only pattern.
if /secrets\.yml\.enc/.match?(error.message)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3088 files inspected, 10 offenses detected, 10 offenses autocorrectable
```
## Additional Information
This behavior change is based on:
https://github.com/rubocop/rubocop-performance/pull/332
Previously, ActionDispatch::IllegalStateError was deprecated using
Module#deprecate_constant in 0b4b4c6b96a41ef649f15e1a3df26e28ef95ff24.
This requires the -w flag to be used to actually see the deprecation
warning, and it can not be controlled using ActiveSupport::Deprecator
configuration.
This commit changes the deprecation to use #deprecate_constant from
ActiveSupport::Deprecation::DeprecatedConstantAccessor. This ensures
that the deprecation warning will be printed even without -w, and the
warning can be controlled by configuring ActionDispatch.deprecator
This error used to be a wrapper for a LoadError raised when
require_dependency was used to load helpers for controllers.
Since Zeitwerk does not use require_dependency, the only usage of the
error was removed in 5b28a0e972da31da570ed24be505ef7958ab4b5e.
It was moved from action_controller/base/helpers.rb to its current
location in 28508d444e36dc8b5819f011f0a2398f44d8d3e3. At that time,
there were only two instances of is_missing being used: one in
action_controller/base/helpers and one in action_mailer/base/helpers.rb.
The action_mailer usage was moved to abstract_controller/helpers in
684c2dc20801b7fcc941ec9478d33d3bf7c74551, and the action_controller
usage moved in 0e063f435ce31a091d1097156172d551bd9d9d37. This last
usage was later removed in 5b28a0e972da31da570ed24be505ef7958ab4b5e,
leaving the require now unused.
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`.
