Skip to content

Commit

Permalink
Fix case sensitive array query parameters (#8988)
Browse files Browse the repository at this point in the history
Express query parser aggreates multiple values per key in an array. Properly canonicalize that case.

Signed-off-by: Jeff Schmidt <jeffrey.schmidt@swirldslabs.com>
  • Loading branch information
Jeff Schmidt authored Aug 9, 2024
1 parent 96acb4c commit b9e2f50
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 5 deletions.
11 changes: 11 additions & 0 deletions hedera-mirror-rest/__tests__/requestHandler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,15 @@ describe('qs tests', () => {
'token.id': ['2', '8', 'gt:4', 'gte:6', '10'],
});
});

test('requestQueryParser for single lowercased query param values', () => {
const val = requestQueryParser('order=ASC&result=SUCCESS');
expect(val.order).toStrictEqual('asc');
expect(val.result).toStrictEqual('success');
});

test('requestQueryParser for multiple lowercased query param keys and values', () => {
const val = requestQueryParser('order=ASC&ORder=ASC');
expect(val.order).toStrictEqual(['asc', 'asc']);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@
{
"urls": [
"/api/v1/transactions?timestamp=1565779209.711927001&account.id=0.0.9&type=credit&result=success&order=asc",
"/api/v1/transactions?timestamp=1565779209.711927001&account.id=0.0.9&type=credit&result=SUCCESS&order=ASC",
"/api/v1/transactions?timestamp=1565779209.711927001&account.id=0.0.9&type=credit&result=SucCess&order=AsC"
"/api/v1/transactions?timestamp=1565779209.711927001&account.id=0.0.9&type=credit&result=SUCCESS&order=ASC&result=SUCCESS&order=ASC",
"/api/v1/transactions?timestamp=1565779209.711927001&account.id=0.0.9&type=credit&result=SucCess&order=AsC&Result=SUCCESS&oRder=ASC"
],
"responseStatus": 200,
"responseJson": {
Expand Down
14 changes: 11 additions & 3 deletions hedera-mirror-rest/middleware/requestHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const requestLogger = async (req, res, next) => {
};

/**
* Manage request query params to support case insensitive keys
* Manage request query params to support case insensitive keys and some values (queryCanonicalizationMap above).
* Express default query parser uses qs, other option is querystring, both are case sensitive
* Parse using default qs logic and use to populate a new map in which all keys are lowercased
* @param queryString
Expand All @@ -62,8 +62,7 @@ const requestQueryParser = (queryString) => {
const caseInsensitiveQueryString = {};
for (const [key, value] of Object.entries(parsedQueryString)) {
const lowerKey = key.toLowerCase();
const canonicalizationFunc = queryCanonicalizationMap[lowerKey];
const canonicalValue = canonicalizationFunc ? canonicalizationFunc(value) : value;
const canonicalValue = canonicalizeValue(lowerKey, value);
if (lowerKey in caseInsensitiveQueryString) {
// handle repeated values, merge into an array
caseInsensitiveQueryString[lowerKey] = merge(caseInsensitiveQueryString[lowerKey], canonicalValue);
Expand All @@ -75,4 +74,13 @@ const requestQueryParser = (queryString) => {
return caseInsensitiveQueryString;
};

const canonicalizeValue = (key, value) => {
const canonicalizationFunc = queryCanonicalizationMap[key];
if (canonicalizationFunc === undefined) {
return value;
}

return Array.isArray(value) ? value.map((v) => canonicalizationFunc(v)) : canonicalizationFunc(value);
};

export {requestLogger, requestQueryParser};

0 comments on commit b9e2f50

Please sign in to comment.