Skip to content

Commit e1d58cf

Browse files
martinemdedari-us
martinemde
and
dari-us
authored
Add Parameters#expect to safely filter and require params (rails#51674)
* `Parameters#expect` safely filters and requires params * `Parameters#expect!` raises unhandled exception on missing params * Add public/400.html file rendered on bad request --------- Co-authored-by: dari-us <[email protected]>
1 parent 1f64e4d commit e1d58cf

File tree

33 files changed

+1031
-153
lines changed

33 files changed

+1031
-153
lines changed

‎actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ def new_mail
3131
end
3232

3333
def mail_params
34-
params.require(:mail).permit(:from, :to, :cc, :bcc, :x_original_to, :in_reply_to, :subject, :body, attachments: [])
34+
params.expect(mail: [:from, :to, :cc, :bcc, :x_original_to, :in_reply_to, :subject, :body, attachments: []])
3535
end
3636

3737
def create_inbound_email(mail)

‎actionmailbox/test/dummy/public/400.html

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>The server cannot process the request due to a client error (400)</title>
5+
<meta name="viewport" content="width=device-width,initial-scale=1">
6+
<style>
7+
.rails-default-error-page {
8+
background-color: #EFEFEF;
9+
color: #2E2F30;
10+
text-align: center;
11+
font-family: arial, sans-serif;
12+
margin: 0;
13+
}
14+
15+
.rails-default-error-page div.dialog {
16+
width: 95%;
17+
max-width: 33em;
18+
margin: 4em auto 0;
19+
}
20+
21+
.rails-default-error-page div.dialog > div {
22+
border: 1px solid #CCC;
23+
border-right-color: #999;
24+
border-left-color: #999;
25+
border-bottom-color: #BBB;
26+
border-top: #B00100 solid 4px;
27+
border-top-left-radius: 9px;
28+
border-top-right-radius: 9px;
29+
background-color: white;
30+
padding: 7px 12% 0;
31+
box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
32+
}
33+
34+
.rails-default-error-page h1 {
35+
font-size: 100%;
36+
color: #730E15;
37+
line-height: 1.5em;
38+
}
39+
40+
.rails-default-error-page div.dialog > p {
41+
margin: 0 0 1em;
42+
padding: 1em;
43+
background-color: #F7F7F7;
44+
border: 1px solid #CCC;
45+
border-right-color: #999;
46+
border-left-color: #999;
47+
border-bottom-color: #999;
48+
border-bottom-left-radius: 4px;
49+
border-bottom-right-radius: 4px;
50+
border-top-color: #DADADA;
51+
color: #666;
52+
box-shadow: 0 3px 8px rgba(50, 50, 50, 0.17);
53+
}
54+
</style>
55+
</head>
56+
57+
<body class="rails-default-error-page">
58+
<!-- This file lives in public/400.html -->
59+
<div class="dialog">
60+
<div>
61+
<h1>The server cannot process the request due to a client error.</h1>
62+
<p>Please check the request and try again.</p>
63+
</div>
64+
<p>If you are the application owner check the logs for more information.</p>
65+
</div>
66+
</body>
67+
</html>

‎actionpack/CHANGELOG.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,50 @@
1+
* Introduce safer, more explicit params handling method with `params#expect` such that
2+
`params.expect(table: [ :attr ])` replaces `params.require(:table).permit(:attr)`
3+
4+
Ensures params are filtered with consideration for the expected
5+
types of values, improving handling of params and avoiding ignorable
6+
errors caused by params tampering.
7+
8+
```ruby
9+
# If the url is altered to ?person=hacked
10+
# Before
11+
params.require(:person).permit(:name, :age, pets: [:name])
12+
# raises NoMethodError, causing a 500 and potential error reporting
13+
14+
# After
15+
params.expect(person: [ :name, :age, pets: [[:name]] ])
16+
# raises ActionController::ParameterMissing, correctly returning a 400 error
17+
```
18+
19+
You may also notice the new double array `[[:name]]`. In order to
20+
declare when a param is expected to be an array of parameter hashes,
21+
this new double array syntax is used to explicitly declare an array.
22+
`expect` requires you to declare expected arrays in this way, and will
23+
ignore arrays that are passed when, for example, `pet: [:name]` is used.
24+
25+
In order to preserve compatibility, `permit` does not adopt the new
26+
double array syntax and is therefore more permissive about unexpected
27+
types. Using `expect` everywhere is recommended.
28+
29+
We suggest replacing `params.require(:person).permit(:name, :age)`
30+
with the direct replacement `params.expect(person: [:name, :age])`
31+
to prevent external users from manipulating params to trigger 500
32+
errors. A 400 error will be returned instead, using public/400.html
33+
34+
Usage of `params.require(:id)` should likewise be replaced with
35+
`params.expect(:id)` which is designed to ensure that `params[:id]`
36+
is a scalar and not an array or hash, also requiring the param.
37+
38+
```ruby
39+
# Before
40+
User.find(params.require(:id)) # allows an array, altering behavior
41+
42+
# After
43+
User.find(params.expect(:id)) # expect only returns non-blank permitted scalars (excludes Hash, Array, nil, "", etc)
44+
```
45+
46+
*Martin Emde*
47+
148
* System Testing: Disable Chrome's search engine choice by default in system tests.
249
350
*glaszig*

0 commit comments

Comments
 (0)