| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
Follow up of #32605.
|
|
|
|
|
| |
This autocorrects the violations after adding a custom cop in
3305c78dcd.
|
| |
|
| |
|
|
|
|
| |
Follow up of #31432.
|
|
|
|
| |
Fixes #31220.
|
|
|
|
|
|
| |
Using the action_dispatch.cookies_rotations interface, key rotation is
now possible with cookies. Thus the secret_key_base as well as salts,
ciphers, and digests, can be rotated without expiring sessions.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since this test changed in 9220935 I noticed that it really doesn't make
sense anymore. I split the tests into 2 groups to explain what each one
does.
First these routes should throw a `bad_request` when the encoding isn't
valid. We're expecting UTF8 encoding and passing binary, that should be
a bad request.
For the second test we are setting the `show` route to set
`self.binary_params_for?` for that route which will convert the
parameters and return a `:ok` instead of a `:bad_request`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit changes the behavior such the path_params now default to
UTF8 just like regular parameters. This also changes the behavior such
that if a path parameter contains invalid UTF8 it returns a 400 bad
request. Previously the behavior was to encode the path params as binary
but that's not the same as query params.
So this commit makes path params behave the same as query params.
It's important to test with a path that's encoded as binary because
that's how paths are encoded from the socket. The test that was altered
was changed to make the behavior for bad encoding the same as query
params. We want to treat path params the same as query params. The params
in the test are invalid UTF8 so they should return a bad request.
Fixes #29669
*Eileen M. Uchitelle, Aaron Patterson, & Tsukuru Tanimichi*
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, raise `BadRequest` if params encoding is invalid.
https://github.com/rails/rails/blob/5-1-stable/actionpack/lib/action_dispatch/http/parameters.rb#L64..L74
https://github.com/rails/rails/blob/5-1-stable/actionpack/lib/action_dispatch/request/utils.rb#L26..L39
However, env values are ensure encoded in ASCII 8 BIT at rack 2.0.3.
https://github.com/rack/rack/commit/68db9aa99e3e2775a58621f658b2a7a0f67db459
Therefore, even if specify an invalid urls, it will not cause an error.
|
|
|
|
|
| |
Since this protection is now in Parameters we can use it instead of
reimplementing again.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The current check for whether to add an optional format to the path
is very lax and will match things like `:format_id` where there are
nested resources, e.g:
resources :formats do
resources :items
end
Fix this by using a more restrictive regex pattern that looks for
the patterns `(.:format)`, `.:format` or `/` at the end of the path.
Note that we need to allow for multiple closing parenthesis since
the route may be of this form:
get "/books(/:action(.:format))", controller: "books"
This probably isn't what's intended since it means that the default
index action route doesn't support a format but we have a test for
it so we need to allow it.
Fixes #28517.
|
|
|
|
|
|
|
|
|
|
| |
In 9b654d4 some params munging was added to ensure that they were
set whenever `recognize_path` would call either a proc or callable
constraint. Since we no longer mutate the environment hash within
the method it's now unnecessary and actually causes params to leak
between route matches before checking constraints.
Fixes #28398.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In ca324a0 the flash middleware was effectively removed by its
constructor returning the app it was passed and the `commit_flash`
call was moved to the `ActionController::Metal#dispatch` method.
This broke any redirect routes that modified the flash because the
redirect happens before `dispatch` gets called.
To fix it, this commit adds a `commit_flash` call in the `serve`
method of `ActionDispatch::Routing::Redirect`.
Fixes #27992.
|
|
|
|
| |
Actually, private methods cannot be called with `self.`, so it's not just redundant, it's a bad habit in Ruby
|
| |
|
| |
|
|
|
|
| |
`ActionDispatch::Routing::Mapper#match`
|
|
|
|
| |
`ActionDispatch::Routing::Mapper#match`
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently a misleading "missing required keys" error is thrown when a param
fails to match the constraints of a particular route. This commit ensures that
these params are recognised as unmatching rather than missing.
Note: this means that a different error message will be provided between
optimized and non-optimized path helpers, due to the fact that the former does
not check constraints when matching routes.
Fixes #26470.
|
|
|
|
|
|
|
|
|
| |
This removes the following warning.
```
./test/dispatch/routing_test.rb:3696: warning: method redefined; discarding old test_namespaced_roots
./test/dispatch/routing_test.rb:1632: warning: previous definition of test_namespaced_roots was here
```
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The PR #20940 enabled the use of multiple roots with different constraints
at the top level but unfortunately didn't work when those roots were inside
a namespace and also broke the use of root inside a namespace after a top
level root was defined because the check for the existence of the named route
used the global :root name and not the namespaced name.
This is fixed by using the name_for_action method to expand the :root name to
the full namespaced name. We can pass nil for the second argument as we're not
dealing with resource definitions so don't need to handle the cases for edit
and new routes.
Fixes #26148.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since e852daa6976cc6b6b28ad0c80a188c06e226df3c only the verb methods
where extracting the defaults options. It was merged a fix for the
`root` method in 31fbbb7faccba25b2e3b5e10b8fca1468579d629 but `match`
was still broken since `:defaults` where not extracted.
This was causing routes defined using `match` and having the `:defaults`
keys to not be recognized.
To fix this it was extracted a new private method with the actual
content of `match` and the `:defaults` extracting was moved to `match`.
|
|
|
|
|
|
|
|
| |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|
| |
|
| |
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
|\
| |
| |
| | |
Fix keyed defaults with root
|
|/
|
|
|
|
|
|
|
| |
The merging of the 'defaults' option was moved up the stack in e852daa
This allows us to see where these options originate from the standard
HttpHelpers (get, post, patch, put, delete)
Unfortunately this move didn't incorporate the 'root' method, which has
always allowed the same 'defaults' option before.
|
|
|
|
|
|
|
|
| |
Check for any non-UTF8 characters in path parameters at the point they're
set in `env`. Previously they were checked for when used to get a controller
class, but this meant routes that went directly to a Rack app, or skipped
controller instantiation for some other reason, had to defend against
non-UTF8 characters themselves.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In Rails 4 these kind of routes used to work:
```ruby
scope '/*id', controller: :builds, as: :build do
get action: :show
end
```
But since 1a830cbd830c7f80936dff7e3c8b26f60dcc371d, routes are only created for
paths specified as strings or symbols. Implicit `nil` paths are just ignored,
with no deprecation warnings or errors. Routes are simply not created. This come
as a surprise for people migrating to Rails 5, since the lack of logs or errors
makes hard to understand where the problem is.
This commit introduces a deprecation warning in case of path as `nil`, while
still allowing the route definition.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Forgotten followup to #23669 :grimacing:
If you went to an internal route (e.g. `/rails/info/routes`), you would
previously see the following in your logger:
```bash
Processing by Rails::InfoController#routes as HTML
Parameters: {"internal"=>true}
Rendering /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application
Rendered collection of /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb [2 times] (10.5ms)
Rendered /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb (2.5ms)
Rendered /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application (23.5ms)
Completed 200 OK in 50ms (Views: 35.1ms | ActiveRecord: 0.0ms)
```
Now, with this change, you would see:
```bash
Processing by Rails::InfoController#routes as HTML
Rendering /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application
Rendered collection of /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_route.html.erb [2 times] (1.6ms)
Rendered /Users/jon/code/rails/rails/actionpack/lib/action_dispatch/middleware/templates/routes/_table.html.erb (10.2ms)
Rendered /Users/jon/code/rails/rails/railties/lib/rails/templates/rails/info/routes.html.erb within layouts/application (17.4ms)
Completed 200 OK in 44ms (Views: 28.0ms | ActiveRecord: 0.0ms)
```
|
|\
| |
| |
| | |
Refactor handling of :action default in routing
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The longstanding convention in Rails is that if the :action parameter
is missing or nil then it defaults to 'index'. Up until Rails 5.0.0.beta1
this was handled slightly differently than other routing defaults by
deleting it from the route options and adding it to the recall parameters.
With the recent focus of removing unnecessary duplications this has
exposed a problem in this strategy - we are now mutating the request's
path parameters and causing problems for later url generation. This will
typically affect url_for rather a named url helper since the latter
explicitly pass :controller, :action, etc.
The fix is to add a default for :action in the route class if the path
contains an :action segment and no default is passed. This change also
revealed an issue with the parameterized part expiry in that it doesn't
follow a right to left order - as soon as a dynamic segment is required
then all other segments become required.
Fixes #23019.
|
|/
|
|
|
|
|
|
| |
Allowing :controller and :action values to be specified via the path
in config/routes.rb has been an underlying cause of a number of issues
in Rails that have resulted in security releases. In light of this it's
better that controllers and actions are explicitly whitelisted rather
than trying to blacklist or sanitize 'bad' values.
|
|
|
|
|
|
|
|
|
|
|
|
| |
The Mapper build_path method marks routes where path parameters are part
of a path segment as custom routes by altering the regular expression, e.g:
get '/foo-:bar', to: 'foo#bar'
There were some edge cases where certain constructs weren't being picked
up and this commit fixes those.
Fixes #23069.
|
|
|
|
|
|
|
|
| |
- Earlier only Hash was allowed as params argument to url_helpers.
- Now ActionController::Parameters instances will also be allowed.
- If the params are not secured then it will raise an ArgumentError to
indicate that constructing URLs with non-secure params is not recommended.
- Fixes #22832.
|
| |
|
| |
|
| |
|
|
|
|
|
| |
also change the feeler to subclass AD::Request so that it has all the
methods that Request has
|
| |
|
|
|
|
|
|
|
|
|
| |
This will silence deprecation warnings.
Most of the test can be changed from `render :text` to render `:plain`
or `render :body` right away. However, there are some tests that needed
to be fixed by hand as they actually assert the default Content-Type
returned from `render :body`.
|
|\
| |
| | |
Respect routing precedence for HEAD requests
|
| |
| |
| |
| |
| |
| |
| | |
Fixes the issue described in #18764 - prevents Rack middleware from
swallowing up HEAD requests that should have been matched by a
higher-precedence `get` route, but still allows Rack middleware to
respond to HEAD requests.
|