Unpredictable response on invalid query parameters
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
neutron |
Fix Released
|
Medium
|
Hongbin Lu |
Bug Description
Problem
=======
If users provide different kinds of invalid query parameters, the behavior of neutron server looks unpredictable. Also Neutron not provide which fields/
it tolerates the invalid inputs and return a successful response. Sometimes, it invalidates the inputs and return an error response.
In addition, the error type is unpredictable and the format of the error message is inconsistent.
1. Invalid sort_key and/or sort_dir:
Neutron returns at least two error types: BadRequest [1] [2] [3] [4], HTTPBadRequest [5] [6] [7] [8]. In addition, the format of error message looks inconsistent. For example, it has the following forms:
* Bad networks request: Attribute '...' references another resource and cannot be used to sort '...' resources. [1]
- Here we need to split whether the attribute fields are valid to sort or Neutron dosen't support it for sorting(maybe the show fields is just foreign constraints from other db tables). [3] [4] [8]
* Bad networks request: '...' is an invalid attribute for sort key. [2]
* [u'...'] is invalid attribute for sort_keys [5]
2. Invalid limit and/or marker:
Sometimes, neutron returns error with one of the following types: BadRequest [9], NetworkNotFound [10]. Sometimes, neutron doesn't return error [11].
3. Invalid filter
Sometimes, neutron returns error with one of the following types: InvalidInput [12], NotImplementedError [13]. Sometimes, neutron doesn't return error [14]
4. Invalid fields
Neutron always returns a successful response [15] regardless of validity of the inputs.
Proposal
========
Improve the predictability of neutron server by handling invalid query parameters consistently. Optimally, neutron server behaves as following:
* Always returns error (i.e. 400 response) on invalid user inputs. IMHO, ignoring invalid inputs (like [11][14][15]) is bad.
* Make error type predictable. There are several options to achieve this. One option is to group different kinds of invalid inputs and define
a specific error type for each group (i.e. Simple split them as InvalidSorting for #1, InvalidPagination for #2, InvalidFilter for #3, etc. Or more sophisticated).
* Make the format of error message consistent. For example, the format of error messages [1][2][3] can be consolidated into one.
Referred examples
=================
[1] GET "/v2.0/
{"NeutronError": {"message": "Bad networks request: Attribute 'segments' references another resource and cannot be used to sort 'networks' resources.", "type": "BadRequest", "detail": ""}}
[2] GET "/v2.0/
{"NeutronError": {"message": "Bad networks request: 'provider:
[3] GET "/v2.0/
{"NeutronError": {"message": "Bad networks request: The attribute 'segments' is reference to other resource, can't used by sort 'networks'.", "type": "BadRequest", "detail": ""}}
[4] GET "/v2.0/
{"NeutronError": {“message”: “Bad networks request: port_security_
[5] GET "/v2.0/
{"NeutronError": {"message": "[u'xxx'] is invalid attribute for sort_keys", "type": "HTTPBadRequest", "detail": ""}}
[6] GET "/v2.0/
{"NeutronError": {"message": "The number of sort_keys and sort_dirs must be same", "type": "HTTPBadRequest", "detail": ""}}
[7] GET "/v2.0/
{"NeutronError": {"message": "[u'xxx'] is invalid value for sort_dirs, valid value is 'asc' and 'desc'", "type": "HTTPBadRequest", "detail": ""}}
[8] GET "/v2.0/
{"NeutronError": {"message": "[u'qos_policy_id'] is invalid attribute for sort_keys", "type": "HTTPBadRequest", "detail": ""}}
[9] GET "/v2.0/
{"NeutronError": {"message": "Bad limit request: Limit must be an integer 0 or greater and not '-1'.", "type": "BadRequest", "detail": ""}}
[10] GET "/v2.0/
{"NeutronError": {"message": "Network xxx could not be found.", "type": "NetworkNotFound", "detail": ""}}
[11] GET "/v2.0/
{"networks":...}
[12] GET "/v2.0/
{"NeutronError": {"message": "Invalid input for operation: 'xxx' cannot be converted to boolean.", "type": "InvalidInput", "detail": ""}}
[13] GET "/v2.0/
{"NotImplemente
[14] GET "/v2.0/
{"networks":...}
[15] GET "/v2.0/
{"networks":...}
description: | updated |
description: | updated |
description: | updated |
description: | updated |
description: | updated |
description: | updated |
description: | updated |
description: | updated |
description: | updated |
Changed in neutron: | |
assignee: | nobody → Hongbin Lu (hongbin.lu) |
Changed in neutron: | |
status: | New → Confirmed |
description: | updated |
Changed in neutron: | |
importance: | Undecided → Medium |
Changed in neutron: | |
milestone: | none → rocky-1 |
Changed in neutron: | |
milestone: | rocky-1 → rocky-2 |
Changed in neutron: | |
milestone: | rocky-2 → rocky-3 |
Changed in neutron: | |
status: | In Progress → Fix Committed |
Changed in neutron: | |
status: | Fix Committed → Fix Released |
The recommended behavior for non-existing marker is explained here: http:// specs.openstack .org/openstack/ api-wg/ guidelines/ pagination_ filter_ sort.html (the last paragraph of "Pagination" section)