Content
View differences
Updated by Tobias Dillmann 17 days ago
###
### Steps to reproduce
...
### What is the buggy behavior?
A client is sending this filter param, the reason is unknown.
```text
"filters": "[[\"status\",\"=\",[\"closed\"]]]",
```
It would be interesting to know where this comes from.
In any case, it makes sense to harden `ParseQueryParamsService#filter_from_params` and validate that each filter element is a Hash. Then raise something that `json_parsed_params` already knows how to rescue. Since `JSON::ParserError` is already rescued there and turned into a `ServiceResult.failure`, raising it (or an `ArgumentError`) on invalid input would produce a proper 422 instead of a 500:
```ruby
# Pseudo-code proposal:
def filter_from_params(filter)
unless filter.is_a?(Hash)
raise JSON::ParserError, "Filter must be a JSON object, got: #{filter.class}"
end
attribute = filter.keys.first
# ...
end
```
<br>
###
###
###
### What is the expected behavior?
...
###
### **Logs**
* Appsignal: [https://appsignal.com/openproject-gmbh/sites/673c529383eb67b55471dda2/exceptions/incidents/1160](https://appsignal.com/openproject-gmbh/sites/673c529383eb67b55471dda2/exceptions/incidents/1160)
```text
undefined method 'keys' for an instance of Array
Backtrace:
app/services/api/v3/parse_query_params_service.rb:138 API::V3::ParseQueryParamsService#filter_from_params
app/services/api/v3/parse_query_params_service.rb:133 block in API::V3::ParseQueryParamsService#filters_from_params
app/services/api/v3/parse_query_params_service.rb:132 Array#map
app/services/api/v3/parse_query_params_service.rb:132 API::V3::ParseQueryParamsService#filters_from_params
app/services/api/v3/parse_query_params_service.rb:53 API::V3::ParseQueryParamsService#json_parsed_params
app/services/api/v3/parse_query_params_service.rb:38 API::V3::ParseQueryParamsService#call
app/services/api/v3/update_query_from_v3_params_service.rb:42 API::V3::UpdateQueryFromV3ParamsService#call
app/services/api/v3/work_package_collection_from_query_service.rb:46 API::V3::WorkPackageCollectionFromQueryService#call
app/services/api/v3/work_package_collection_from_query_params_service.rb:44 API::V3::WorkPackageCollectionFromQueryParamsService#call
lib/api/v3/work_packages/work_packages_api.rb:52 block (3 levels) in <class:WorkPackagesAPI>
lib/api/root_api.rb:262 API::RootAPI::Helpers#raise_invalid_query_on_service_failure
lib/api/v3/work_packages/work_packages_api.rb:49 block (2 levels) in <class:WorkPackagesAPI>
```
### Steps to reproduce
...
### What is the buggy behavior?
A client is sending this filter param, the reason is unknown.
```text
"filters": "[[\"status\",\"=\",[\"closed\"]]]",
```
It would be interesting to know where this comes from.
In any case, it makes sense to harden `ParseQueryParamsService#filter_from_params` and validate that each filter element is a Hash. Then raise something that `json_parsed_params` already knows how to rescue. Since `JSON::ParserError` is already rescued there and turned into a `ServiceResult.failure`, raising it (or an `ArgumentError`) on invalid input would produce a proper 422 instead of a 500:
```ruby
# Pseudo-code proposal:
def filter_from_params(filter)
unless filter.is_a?(Hash)
raise JSON::ParserError, "Filter must be a JSON object, got: #{filter.class}"
end
attribute = filter.keys.first
# ...
end
```
<br>
###
###
###
### What is the expected behavior?
...
###
### **Logs**
* Appsignal: [https://appsignal.com/openproject-gmbh/sites/673c529383eb67b55471dda2/exceptions/incidents/1160](https://appsignal.com/openproject-gmbh/sites/673c529383eb67b55471dda2/exceptions/incidents/1160)
```text
undefined method 'keys' for an instance of Array
Backtrace:
app/services/api/v3/parse_query_params_service.rb:138 API::V3::ParseQueryParamsService#filter_from_params
app/services/api/v3/parse_query_params_service.rb:133 block in API::V3::ParseQueryParamsService#filters_from_params
app/services/api/v3/parse_query_params_service.rb:132 Array#map
app/services/api/v3/parse_query_params_service.rb:132 API::V3::ParseQueryParamsService#filters_from_params
app/services/api/v3/parse_query_params_service.rb:53 API::V3::ParseQueryParamsService#json_parsed_params
app/services/api/v3/parse_query_params_service.rb:38 API::V3::ParseQueryParamsService#call
app/services/api/v3/update_query_from_v3_params_service.rb:42 API::V3::UpdateQueryFromV3ParamsService#call
app/services/api/v3/work_package_collection_from_query_service.rb:46 API::V3::WorkPackageCollectionFromQueryService#call
app/services/api/v3/work_package_collection_from_query_params_service.rb:44 API::V3::WorkPackageCollectionFromQueryParamsService#call
lib/api/v3/work_packages/work_packages_api.rb:52 block (3 levels) in <class:WorkPackagesAPI>
lib/api/root_api.rb:262 API::RootAPI::Helpers#raise_invalid_query_on_service_failure
lib/api/v3/work_packages/work_packages_api.rb:49 block (2 levels) in <class:WorkPackagesAPI>
```