Fix case sensitive query parameters #8852
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rameter values. Introduce small framework to extend to other query parameter keys as required. Signed-off-by: Jeff Schmidt <jeffrey.schmidt@swirldslabs.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8852 +/- ##
=========================================
Coverage 92.19% 92.19%
- Complexity 7597 7598 +1
=========================================
Files 925 925
Lines 30407 30427 +20
Branches 3694 3706 +12
=========================================
+ Hits 28033 28053 +20
Misses 1548 1548
Partials 826 826 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Xin Li <xin@swirldslabs.com>
steven-sheehy
changed the title
|
steven-sheehy
deleted the
8790-some-rest-api-endpoints-return-500-with-order=asc-order=desc
branch
bilyana-gospodinova
pushed a commit
that referenced
this pull request
Jul 24, 2024
Some rest api endpoints return 500 with `order=ASC` / `order=DESC`, examples: - `https://mainnet-public.mirrornode.hedera.com/api/v1/contracts/results/logs?order=ASC` - `https://mainnet-public.mirrornode.hedera.com/api/v1/accounts/0.0.1274288/nfts?limit=100&order=ASC` The transactions REST API misinterprets query filter `result=SUCCESS` to look for transactions with response code not equal to 22. The issue is only all lower case `success` is interpreted as response code equal to 22. On top of the inverted interpretation, this may also cause query timeout, e.g., when combined with `account.id` for which there's very few failed transactions. This PR, in the `requestHander.js` middleware Express query parser, ensures the query parameter values to selected parameters (currently just `order` and `result`) are forced to lowercase before the request is routed to the request handler code. --------- Signed-off-by: Jeff Schmidt <jeffrey.schmidt@swirldslabs.com> Signed-off-by: Xin Li <xin@swirldslabs.com> Co-authored-by: Xin Li <xin@swirldslabs.com>
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Some rest api endpoints return 500 with
order=ASC/order=DESC, examples:https://mainnet-public.mirrornode.hedera.com/api/v1/contracts/results/logs?order=ASChttps://mainnet-public.mirrornode.hedera.com/api/v1/accounts/0.0.1274288/nfts?limit=100&order=ASCThe transactions REST API misinterprets query filter
result=SUCCESSto look for transactions with response code not equal to 22. The issue is only all lower casesuccessis interpreted as response code equal to 22. On top of the inverted interpretation, this may also cause query timeout, e.g., when combined withaccount.idfor which there's very few failed transactions.This PR, in the
requestHander.jsmiddleware Express query parser, ensures the query parameter values to selected parameters (currently justorderandresult) are forced to lowercase before the request is routed to the request handler code.Related issue(s):
Fixes #8790
Fixes #8801
Notes for reviewer:
Checklist