| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
| | | | |
|
|\| | | |
|
| | | | |
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
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.
|
| |\ \ \
| | | | |
| | | | |
| | | | |
| | | | | |
eileencodes/force-encoding-to-original-string-encoding
Maintain original encoding from path
|
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
When the path info is read from the socket it's encoded as ASCII 8BIT.
The unescape method changes the encoding to UTF8 but it should maintain
the encoding of the string that's passed in.
This causes parameters to be force encoded to UTF8 when we don't
actually know what the encoding of the parameter should be.
|
| |/ / /
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
In order to fully support the same interface as `Hash#delete`, we need
to pass the block through to the underlying method, not just the key.
This used to work correctly, but it regressed when
`ActionController::Parameters` stopped inheriting from `Hash` in 5.0.
|
| | |/
| |/| |
|
|\| | |
|
| | | |
|
| | |
| | |
| | |
| | |
| | | |
Closes #28382
Closes #28651
|
| | |
| | |
| | |
| | |
| | | |
Since this protection is now in Parameters we can use it instead of
reimplementing again.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Previously it was raising an error because it may be unsafe to use those
methods in a unpermitted parameter. Now we delegate to to_h that already
raise an error when the Parameters instance is not permitted.
This also fix a bug when using `#to_query` in a hash that contains a
`ActionController::Parameters` instance and was returning the name of the
class in the string.
|
| | | |
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Now methods that implicit convert objects to a hash will be able to work
without requiring the users to change their implementation.
This method will return a Hash instead of a HashWithIndefirentAccess
to mimic the same implementation of HashWithIndefirentAccess#to_hash.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Before we returned either an empty hash or only the always permitted
parameters (:controller and :action by default).
The previous behavior was dangerous because in order to get the
attributes users usually fallback to use to_unsafe_h that could
potentially introduce security issues.
The to_unsafe_h API is also not good since Parameters is a object that
quacks like a Hash but not in all cases since to_h would return an empty
hash and users were forced to check if to_unsafe_h is defined or if the
instance is a ActionController::Parameters in order to work with it.
This end up coupling a lot of libraries and parts of the application
with something that is from the controller layer.
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Effectively treat nil values as "auto", e.g. whatever a form helper
chooses to interpret it as.
But treat an explicitly assigned false value as disabling.
|
| |/
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I came up against this while dealing with a misconfigured server. The
browser was setting the Origin header to "https://example.com", but the
Rails app returned "http://example.com" from request.base_url (because
it was failing to detect that HTTPS was used).
This caused verify_authenticity_token to fail, but the message in the
log was "Can't verify CSRF token", which is confusing because the
failure had nothing to do with the CSRF token sent in the request. This
made it very hard to identify the issue, so hopefully this will make it
more obvious for the next person.
|
|\| |
|
| |\
| | |
| | | |
Do not include default response headers for AC::Metal
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
In Rails 4.2, `ActionController::Metal` controllers did not include the
default headers from `ActionDispatch::Response`. However, through e16afe6, and a
general shift towards having `ActionController::Metal` objects contain
`ActionDispatch::Response` objects (instead of just returning an array
of status, headers, and body), this behavior was lost. This PR helps to
restore the original behavior by having `ActionController::Metal`
controllers generate Response objects without the default headers, while
`ActionController::Base` now overrides the factory method to make sure
its version does have the default headers.
|
| |\ \
| | |/
| |/| |
Add an alias for reverse_merge to with_defaults
|
| | |
| | |
| | |
| | |
| | |
| | | |
In the context of controller parameters, reverse_merge is commonly used
to provide defaults for user input. Having an alias to reverse_merge
called with_defaults feels more idiomatic for Rails.
|
| |/ |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Sometimes we want to use rack_test partially instead of selenium for test speed:
```ruby
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
driven_by :selenium, using: :chrome, screen_size: [1400, 1400], options: {url: "http://chrome:4444/wd/hub"}
end
class WithJavaScriptTest < ApplicationSystemTestCase
end
class WithoutJavaScriptTest < ApplicationSystemTestCase
driven_by :rack_test
end
```
In the abobe case, `WithoutJavaScriptTest` uses selenium because
`SystemTestCase` calls superclass' driver on `#initialize` (`self.class.superclass.driver.use`).
Using `class_attribute` can handle inherited `driven_by`.
|
| |\
| | |
| | | |
Fix store accessors in parameters test
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
* The method name must be `stored_attributes`, not `stores_attributes`.
* `attribute_names` must return a non-empty value. Because
`stored_attributes` is not checked if `attribute_names` is empty.
Follow up to #28056
|
| |/
| |
| |
| | |
Closes #28554
|
| |\
| | |
| | | |
Wrap stored accessors in parameters
|
| | |
| | |
| | |
| | | |
Modified params wrapper to account for model's stored_attributes
|
| | | |
|
| |/ |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This reverts commit c6f9f8c28a720ad4ec7cf3613dddfa451d5968e2, reversing
changes made to c309073c7476f50dfb1e796d058580f176101c36.
Reason: This is fixing the behavior in the wrong place. Now the request
path after the request is nil and there is no way to assert that.
Also the test that was added in that PR also fails in 4.2 where the
reporter says it was passing. The reason the bahavior changed between
Rails 4.2 and Rails 5 is that the format in the path is now respected.
The correct way to fix the problem is not doign two requests in the same
controller test and use integrations tests. This change caused a
regression between Rails 5.0.1 and 5.0.2.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Not all requirements can be expressed in terms of polymorphic url
options so add a `route_for` method that allows calling another
direct route (or regular named route) which a set of arguments, e.g:
resources :buckets
direct :recordable do |recording|
route_for(:bucket, recording.bucket)
end
direct :threadable do |threadable|
route_for(:recordable, threadable.parent)
end
This maintains the context of the original caller, e.g.
threadable_path(threadable) # => /buckets/1
threadable_url(threadable) # => http://example.com/buckets/1
|
| |\
| | |
| | | |
Pass options to `driven_by`
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Capybara drivers can handle some options such like `url`.
### before
```
# test/test_helper.rb
Capybara.register_driver :remote_chrome do |app|
Capybara::Selenium::Driver.new(app, browser: :chrome, url: "http://example.com/wd/hub")
end
# test/application_system_test_case.rb
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
driven_by :remote_chrome
end
```
### after
```
# test/application_system_test_case.rb
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
driven_by :selenium, using: :chrome, screen_size: [1400, 1400], options: {url: "http://chrome:4444/wd/hub"}
end
```
|
| |/
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |
| |
| |
| | |
`env` is undefined.
|
| | |
|
| |\
| | |
| | |
| | |
| | |
| | | |
Added `reverse_merge`/`reverse_merge!` to AC::Parameters:
[Rafael Mendonça França + Mitsutaka Mimura]
|
| | |
| | |
| | |
| | |
| | | |
- This PR adds the `reverse_merge` and `reverse_merge!` method to `ActionController::Parameters`
- Fixes #28353
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Previously the system test subclasses would call `driven_by` when the
app booted and not again when the test was initialized which resulted in
the driver from whichever class was called last to be used in tests.
In rails/rails#28144 the `driven_by` method was changed to run `use` on
setup and `reset` on teardown. While this was a viable fix this really
pointed to the problem that system test `driven_by` was a global
setting, rather than a per-class setting.
To alieviate this problem calling the driver should be done on an
instance level, rather than on the global level. I added an `initialize`
method to `SystemTestCase` which will call `use` on the superclass
driver. Running the server has been moved to `start_application` so that
it only needs to be called once on boot and no options from `driven_by`
were being passed to it.
This required a largish rewrite of the tests. Each test needs to utilize
the subclass so that it can properly test the drivers.
`ActionDispatch::SystemTestCase` shouldn't be called directly anymore.
|
| |/
| |
| |
| |
| |
| |
| | |
Since using a browser is only for selenium it doesn't really make sense
to have a separate class for handling it there. This brings a lot of the
if/else out of the main SystemTestCase class and into the Driver class
so we can abstract away all that extra work.
|
| | |
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`Capybara::RackTest::Driver` does not support taking screenshots. If call
`#save_screenshot` on `Capybara::RackTest::Driver` will raise the error.
```ruby
Error:
UsersTest#test_visiting_the_index:
Capybara::NotSupportedByDriverError: Capybara::Driver::Base#save_screenshot
```
To prevent errors, if driver does not support screenshot, do not call it.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|