| Commit message (Collapse) | Author | Age | Files | Lines |
|\
| |
| |
| |
| | |
greysteil/dont-raise-unknown-http-method-low-in-stack
Don't raise ActionController::UnknownHttpMethod from ActionDispatch::Static
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The `ActionDispatch::Static` middleware is used low down in the stack to serve
static assets before doing much processing. Since it's called from so low in
the stack, we don't have access to the request ID at this point, and generally
won't have any exception handling defined (by default `ShowExceptions` is added
to the stack quite a bit higher and relies on logging and request ID).
Before https://github.com/rails/rails/commit/8f27d6036a2ddc3cb7a7ad98afa2666ec163c2c3
this middleware would ignore unknown HTTP methods, and an exception about these
would be raised higher in the stack. After that commit, however, that exception
will be raised here.
If we want to keep `ActionDispatch::Static` so low in the stack (I think we do)
we should suppress the `ActionController::UnknownHttpMethod` exception here,
and instead let it be raised higher up the stack, once we've had a chance to
define exception handling behaviour.
This PR updates `ActionDispatch::Static` so it passes `Rack::Request` objects to
`ActionDispatch::FileHandler`, which won't raise an
`ActionController::UnknownHttpMethod` error. If an unknown method is
passed, it should exception higher in the stack instead, once we've had a
chance to define exception handling behaviour.`
|
|\ \
| | |
| | |
| | |
| | | |
javan/fix-namespaced-implicit-render-etag-template-digest
Fix adding implicitly rendered namespaced template digests to ETags
|
| |/ |
|
|\ \
| |/
|/| |
Let TestResponse assign a parser.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Previously we'd only assign a response parser when a request came through
Action Dispatch integration tests. This made calls to `parsed_body` when a TestResponse
was manually instantiated — though own doing or perhaps from a framework — unintentionally
blow up because no parser was set at that time.
The response can lookup a parser entirely through its own ivars. Extract request encoder to
its own file and assume that a viable content type is present at TestResponse instantiation.
Since the default response parser is a no-op, making `parsed_body` equal to `body`, no
exceptions will be thrown.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Rack [recently](https://github.com/rack/rack/commit/7e7a3890449b5cf5b86929c79373506e5f1909fb)
moved the namespace of its `ParameterTypeError` and `InvalidParameterError`
errors. Whilst an alias for the old name was added, the logic in
`ActionDispatch::ExceptionWrapper` was still broken by this change, since it
relies on the class name.
This PR updates `ActionDispatch::ExceptionWrapper` to handle the Rack 2.0
namespaced errors correctly. We no longer need to worry about the old names,
since Rails specifies Rack ~> 2.0.
|
| |
| |
| |
| |
| | |
- Tests for dup'ing params was separately added in a separate file in
https://github.com/rails/rails/pull/25735.
|
|\ \
| |/
|/| |
Stop changes to a dupped `ActionController::Parameters` mutating the original
|
| |
| |
| |
| |
| |
| | |
`#initialize_copy` to manually duplicate the underlying parameters hash
It looks like `ActionController::Parameters#dup` is leftover from when the class inherited from `Hash`. We can just trust `#dup`, which already copies the `@permitted` instance variable (confirmed by tests). We still define a `#initialize_copy` to make `@parameters` a copy that can be mutated without affecting the original instance.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When `ActionController::Parameters` is duplicated with `#dup`, it doesn't create a duplicate of the instance variables (e.g. `@parameters`) but rather maintains the reference (see <http://ruby-doc.org/core-2.3.1/Object.html>). Given that the parameters object is often manipulated as if it were a hash (e.g. with `#delete` and similar methods), this leads to unexpected behaviour, like the following:
```
params = ActionController::Parameters.new(foo: "bar")
duplicated_params = params.dup
duplicated_params.delete(:foo)
params == duplicated_params
```
This fixes the bug by defining a private `#initialize_copy` method, used internally by `#dup`, which makes a copy of `@parameters`.
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In response_test.rb, we haven't had a test to make sure that
1) these responses don't have a message-body as described in RFC7231[1]
2) 1xx and 204 responses must not have a Content-Length header field
as described in RFC7230-section3.3.2[2]
[1] https://tools.ietf.org/html/rfc7231
[2] https://tools.ietf.org/html/rfc7230#section-3.3.2
Even though our implementation doesn't allow users to send
a Content-Length header field in a 304 response, sending the
header field is valid as mentioned in RFC7230-section3.3.2[2].
So I've decided not to test whether or not a 304 response has
the header.
The citation from the section is as follows;
```
A server MAY send a Content-Length header field in a 304 (Not
Modified) response to a conditional GET request (Section 4.1 of
[RFC7232]); a server MUST NOT send Content-Length in such a response
unless its field-value equals the decimal number of octets that would
have been sent in the payload body of a 200 (OK) response to the same
request.
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|\
| |
| | |
Renamed NestedParametersTest to NestedParametersPermitTest
|
| |
| |
| |
| | |
what we are actually testing in this file
|
| | |
|
|\ \
| | |
| | | |
Fix setting route's to in a scope
|
| | |
| | |
| | |
| | |
| | |
| | | |
Fixes #25488
97d7dc4 introduced a regression that resulted in ArgumentError when to
was in options of the scope and not of particular route.
|
|/ /
| |
| |
| | |
Signed-off-by: Jeremy Daer <jeremydaer@gmail.com>
|
|\ \
| | |
| | | |
Modifies mime-registration test not to interfere with real mime types
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The tests introduced in
https://github.com/rails/rails/pull/23816/files#diff-384a5a15d8d53de799fb6541688ea5f9R153
register the JSON API media type `application/vnd.api+json` with
`Mime[:json]`. The JSON API media type should not be registered
with `Mime[:json]`, as discussed in #23712. Moreover,
since the actual mime type used in the test is
incidental, I've changed this to a valid, but fictional
`applcation/vnd.rails+json`.
These tests were causing failures in
https://github.com/rails/rails/pull/25050#issuecomment-221092934 where
`Mime[:jsonapi]` is being added, so that JSON API request params are parsed
with the JSONAPI gem.
|
|\ \ \
| | | |
| | | | |
make `as` option work with get parameters
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Currently, if path is a relative path, add format without the discrimination of the query.
Therefore, if there is a query, format at end of the query would been added,
format was not be specified correctly.
This fix add format to end of path rather than query.
|
|/ / /
| | |
| | |
| | |
| | | |
In the docs: "+permit_all_parameters+ - If it's +true+, all the parameters will
be permitted by default. The default is +false+."
|
| | |
| | |
| | |
| | |
| | |
| | | |
* Restore the functionality of PR#14129, but do so with not nil to better indicate the purpose of the conditional
* Add a test when render_to_string called on ActionController::Base.new()
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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)
```
|
| | |
| | |
| | |
| | |
| | | |
Was worried the `as` might impede on users doing the long form
JSON response encoding; test for certainty.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Since 69009f, `ActionController::Metal::DataStreaming#send_file` doesn't
set `@_response_body` anymore.
`AbstractController::Callbacks` used `@_response_body` in its callback
terminator, so it failed to halt the callback cycle when using `#send_file`
from a `before_action`.
Instead, it now uses `#performed?` on `AbstractController::Base` and
`ActionController::Metal`, which checks `response.committed?`, besides
checking if `@_response_body` is set, if possible.
Example application: https://gist.github.com/jeffkreeftmeijer/78ae4572f36b198e729724b0cf79ef8e
|
|\ \ \
| | | |
| | | | |
fix typo [ci skip]
|
| | | | |
|
| | | | |
|
|/ / /
| | |
| | |
| | |
| | | |
- Remove dead classes / dead code
- Move class definitions to where they are used, don't define in a
shared space
|
|/ /
| |
| |
| |
| | |
We should define it only where we need it, not in the global abstract
unit :grimacing:
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
CSRF verification for non-XHR GET requests (cross-origin `<script>`
tags) didn't check this flag before logging failures.
Setting `config.action_controller.log_warning_on_csrf_failure = false`
now disables logging for these CSRF failures as well.
Closes #25086.
Signed-off-by: Jeremy Daer <jeremydaer@gmail.com>
|
| |
| |
| |
| |
| |
| |
| |
| | |
Ruby 2.4 unifies Fixnum and Bignum into Integer: https://bugs.ruby-lang.org/issues/12005
* Forward compat with new unified Integer class in Ruby 2.4+.
* Backward compat with separate Fixnum/Bignum in Ruby 2.2 & 2.3.
* Drops needless Fixnum distinction in docs, preferring Integer.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Follows the same pattern as controllers and jobs. Exceptions raised in
delivery jobs (enqueued by `#deliver_later`) are also delegated to the
mailer's rescue_from handlers, so you can handle the DeserializationError
raised by delivery jobs:
```ruby
class MyMailer < ApplicationMailer
rescue_from ActiveJob::DeserializationError do
…
end
```
ActiveSupport::Rescuable polish:
* Add the `rescue_with_handler` class method so exceptions may be
handled at the class level without requiring an instance.
* Rationalize `exception.cause` handling. If no handler matches the
exception, fall back to the handler that matches its cause.
* Handle exceptions raised elsewhere. Pass `object: …` to execute
the `rescue_from` handler (e.g. a method call or a block to
instance_exec) against a different object. Defaults to `self`.
|
| | |
|
|\ \
| | |
| | | |
Improve documentation and tests for raw_host_with_port and host_with_…
|
| | | |
|
|\ \ \
| | | |
| | | | |
API only apps: Preserve request format for HTML requests too
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
- Earlier we were responding with JSON format for HTML requests in a API
app.
- Now we will respond with HTML format for such requests in API apps.
- Also earlier we were not testing the API app's JSON requests
properly. We were actually sending HTML requests. Now we send correct
JSON requests. Also added more test coverage.
- Based on the discussion from this commit -
https://github.com/rails/rails/commit/05d89410bf97d0778e78558db3c9fed275f8a614.
[Prathamesh Sonpatki, Jorge Bejar]
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
In #18721 we removed the discard key from the session hash used to flash
messages and that broke compatibility with Rails 4 applications because they
try to map in the discarded flash messages and it returns nil.
Fixes #24726.
|
|\ \ \ \
| | | | |
| | | | | |
BoomerAPI is not used anywhere, so removed it!
|
| |/ / /
| | | |
| | | |
| | | |
| | | | |
- It was originally added in 83b4e9073f0852afc065 and partially
removed in 05d89410bf97d0778e7.
|
|\ \ \ \
| |/ / /
|/| | |
| | | |
| | | |
| | | |
| | | | |
rthbound/dont-call-each-when-calling-body-on-response
Dont call each when calling body on response to fix #23964
Fixes #23964
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
- Adds #each_chunk to ActionDispatch::Response. it's a method which
will be called by ActionDispatch::Response#each.
- Make Response#each a proper method instead of delegating to @stream
- In Live, instead of overriding #each, override #each_chunk.
- `#each` should just spit out @str_body if it's already set
- Adds #test_set_header_after_read_body_during_action
to prove this fixes #23964
- Adds #test_each_isnt_called_if_str_body_is_written to
ensure #each_chunk is not called when @str_body is available
- Call `@response.sent!` in AC::TestCase's #perform so a test response
acts a bit more like a real response. Makes test that call `#assert_stream_closed`
pass again.
- Additionally assert `#committed?` in `#assert_stream_closed`
- Make test that was calling @response.stream.each pass again by
calling @response.each instead.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
It is a common pattern in the Rails community that when people want to
:xa
use any kind of helper that is defined inside app/helpers they includes
the helper module inside the controller like:
module UserHelper
def my_user_helper
# ...
end
end
class UsersController < ApplicationController
include UserHelper
def index
render inline: my_user_helper
end
end
This has problem because the helper can't access anything that is
defined in the view level context class.
Also all public methods of the helper become available in the controller
what can lead to undesirable methods being routed and behaving as
actions.
Also if you helper depends on other helpers or even Action View helpers
you need to include each one of these dependencies in your controller
otherwise your helper is not going to work.
We already have a helpers proxy at controller class level but that proxy
doesn't have access to the instance variables defined in the
controller.
With this new instance level helper proxy users can reuse helpers in the
controller without having to include the modules and with access to
instance variables defined in the controller.
class UsersController < ApplicationController
def index
render inline: helpers.my_user_helper
end
end
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Adding the `each` method is required for ensuring compatibility between
Rails, and other Rack frameworks (like Sinatra, etc.), that are mounted
within Rails, and wish to use its session tooling. Prior to this, there
was an inconsistency between ActionDispatch::Request::Session and
Rack::Session::Cookie, due to the absence of the `each` method. This
should hopefully fix that error. :)
For a full integration test with Sinatra and a standalone Rack
application, you can check out the gist for that here: https://gist.github.com/maclover7/08cd95b0bfe259465314311941326470.
Solves #15843.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Last August (2015), @tenderlove worked to remove all `@env[]` and `@env[]=`, in
favor of using `set_header`, `get_header`, etc. (Here's an [example
commit](https://github.com/rails/rails/commit/f16a33b68efc3dc57cfafa27651b9a765e363fbf)).
This PR should remove the last uses of these methods, and fully convert
them to the newly standardized API.
|