Validate header parameters

[tzmfreedom]

On current version of committee, parameters in header is not validated when parameter is defined in header on OpenAPI 2.0 format.

So the following OpenAPI description is not working because current version does't extract and validate header values.

...
properties:
  name:
    type: string
    in: header
...

This pull request solve them.

• If allow_header_params is enabled, header values is extracted as parameter.
• On OpenAPI driver, header parameter is uppercased because we treat header key as CGI header values, that is uppercased and converted hyphen to underscore.

[brandur]

Hi Makoto,

Sorry about the very delayed review here :/ and thank you for the work.

I left a couple comments below.

[brandur]

Can we just perform the upcase when doing the validation? I think having all the logic in one place is just going to make everything in a little more readable.

[brandur]

What do you think about having allow_header_params on by default? As you noticed, it seems to be part of the spec, and the reason that allow_query_params and allow_form_params exist is that they're doing something that's a little abnormal from the standard Committee conventions. Header parameters are almost entirely orthogonal to other types of parameters, and I suspect it's fine just to always allow them.

[tzmfreedom]

Thank you for your pointing it out.
I think allow_header_params should be true by default too.
In the same way as other parameter, I'll set default parameter on RequestValidation's constructor.

[brandur]

I'm a tad bit concerned about the potential for different types of parameters to start colliding. For example, you could have a param called Encoding that appeared as both a header and as a form parameter, and Committee would have a hard time differentiating between the two.

What do you think about separating headers out into separate buckets? Maybe the best solution would be if RequestUnpacker returned a [headers, params] where each element could be validated separately, but a cheaper solution might be to just have a special __committee_headers key in the params hash for now that's extracted during validation. Thoughts?

[tzmfreedom]

As you pointed out, I think header and parameter are orthogonal too.
In current PR implementation, collision is occured and this is not good for us.
So separating headers out into separate buckets is good idea.

I think the best solution is separeting params and headers on RequestUnpacker#call too.

The following is idea for implementation.

• Committee::Drivers::OpenAPI2::Link has new header_schema attribute that is JsonSchema::Schema Object to validate header.
  • Committee::Drivers::HyperSchema does't have header_schema. This attribute only be in OpenAPI.
• RequestUnpacker returns array, params and headers.
• RequestValidator validates params and headers separately. params are validated with Link#schema, headers are validated with Link#header_schema.

If the above solution looks good, I'll implement it.

[brandur]

Thanks for thinking it through so thoroughly! Yes, that sounds good to me.

[tzmfreedom]

I revised.
2eeec41

And, I fixed some test because of redundant variable assignment.
719db39

Please review it and let me know if there are any comments.

[brandur]

Thank you for the significant chunk of work here @tzmfreedom! Once again, apologies for the delay in getting back to you on it — if it happens again in the future, feel free to bump the thread to get it back to the top of my inbox.

This looks good me!

[brandur]

Released as 2.1.0.