Skip to content

Commit 2792ce5

Browse files
authored
Merge pull request #30639 from github/repo-sync
Repo sync
2 parents 760f640 + c5d2af5 commit 2792ce5

File tree

2 files changed

+21
-3
lines changed

2 files changed

+21
-3
lines changed

src/shielding/middleware/handle-invalid-query-string-values.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ const RECOGNIZED_VALUES_KEYS = new Set(Object.keys(RECOGNIZED_VALUES))
3232
export default function handleInvalidQuerystringValues(req, res, next) {
3333
const { method, query } = req
3434
if (method === 'GET' || method === 'HEAD') {
35-
for (const key of Object.keys(query)) {
35+
for (const [key, value] of Object.entries(query)) {
3636
if (RECOGNIZED_VALUES_KEYS.has(key)) {
3737
const validValues = RECOGNIZED_VALUES[key]
3838
const values = Array.isArray(query[key]) ? query[key] : [query[key]]
@@ -66,6 +66,17 @@ export default function handleInvalidQuerystringValues(req, res, next) {
6666
return
6767
}
6868
}
69+
70+
// For example ?foo[bar]=baz (but not ?foo=bar&foo=baz)
71+
if (value instanceof Object && !Array.isArray(value)) {
72+
const message = `Invalid query string key (${key})`
73+
defaultCacheControl(res)
74+
res.status(400).send(message)
75+
76+
const tags = ['response:400', `path:${req.path}`, `key:${key}`]
77+
statsd.increment(STATSD_KEY, 1, tags)
78+
return
79+
}
6980
}
7081
}
7182

src/shielding/tests/invalid-querystrings.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,18 @@ describe('invalid query strings', () => {
5757
}
5858
})
5959

60+
test('query string keys with single square brackets', async () => {
61+
const url = `/en?query[foo]=bar`
62+
const res = await get(url)
63+
expect(res.statusCode).toBe(400)
64+
expect(res.body).toMatch('Invalid query string key (query)')
65+
})
66+
6067
test('query string keys with square brackets', async () => {
6168
const url = `/?constructor[foo][bar]=buz`
6269
const res = await get(url)
63-
expect(res.statusCode).toBe(302)
64-
expect(res.headers.location).toBe('/en')
70+
expect(res.statusCode).toBe(400)
71+
expect(res.body).toMatch('Invalid query string key (constructor)')
6572
})
6673

6774
test('bad tool query string with Chinese URL-encoded characters', async () => {

0 commit comments

Comments
 (0)