On 06/15/2017 10:01 PM, Matt Riedemann wrote: > On 6/15/2017 8:43 PM, Alex Xu wrote: >> We added new decorator 'query_schema' to support validate the query >> parameters by JSON-Schema. >> >> It provides more strict valiadation as below: >> * set the 'additionalProperties=False' in the schema, it means that >> reject any invalid query parameters and return HTTPBadRequest 400 to >> the user. >> * use the marco function 'single_param' to declare the specific query >> parameter only support single value. For example, the 'marker' >> parameters for the pagination actually only one value is the valid. If >> the user specific multiple values "marker=1&marker=2", the validation >> will return 400 to the user. >> >> Currently there is patch related to this: >> https://review.openstack.org/#/c/459483/13/nova/api/openstack/compute/schemas/server_migrations.py >> >> >> So my question is: >> Are we all good with this strict validation in all the future >> microversion? >> >> I didn't remember we explicit agreement this at somewhere, just want >> to double check this is the direction everybody want to go. >> >> Thanks >> Alex >> >> >> __________________________________________________________________________ >> >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: >> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> > > I think this is fine and makes sense for new microversions. The spec for > consistent query parameter validation does talk about it a bit: > > https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/consistent-query-parameters-validation.html#proposed-change > > > "The behaviour additionalProperties as below: > > * When the value of additionalProperties is True means the extra query > parameters are allowed. But those extra query parameters will be > stripped out. > * When the value of additionalProperties is False means the extra query > aren’t allowed. > > The value of additionalProperties will be True until we decide to > restrict the parameters in the future, and it will be changed with new > microversion." > > I don't see a point in allowing someone to specify a query parameter > multiple times if we only pick the first one from the list and use that.
Agreed. The point of doing strict validation and returning a 400 is to help the user eliminate bugs in their program. If they specified marker twice either they thought it did something, or they made a mistake. Both are wrong. When we are silent on that front it means they may not be getting the behavior they were expecting, which hurts their experience with the API. -Sean -- Sean Dague http://dague.net __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev