Validate the headers returned against the schema specification. #459
Conversation
|
Thanks for the PR. Can you also update the documentation?(https://github.com/zalando/connexion/blob/master/docs/request.rst#request-validation) |
connexion/decorators/response.py
Outdated
| @@ -56,6 +56,14 @@ def validate_response(self, data, status_code, headers, url): | |||
| msg = ("Keys in header don't match response specification. " | |||
| "Difference: {0}").format(pretty_list) | |||
| raise NonConformingResponseHeaders(message=msg) | |||
| # validate each of the existing keys. | |||
| for key, schema in response_definition.get("headers").items(): | |||
| v = ResponseHeaderValidator(schema) | |||
There was a problem hiding this comment.
Please rename v to something like validator or header_validator.
|
Updated as per feedback (except I updated |
connexion/decorators/validation.py
Outdated
| error=exception)) | ||
| six.reraise(*sys.exc_info()) | ||
|
|
||
| return None |
There was a problem hiding this comment.
Please remove this line.
| # validate each of the existing keys. | ||
| for key, schema in response_definition.get("headers").items(): | ||
| header_validator = ResponseHeaderValidator(schema) | ||
| for data in headers.getlist(key): |
There was a problem hiding this comment.
The .getlist method is Flask specific. We are supposed to clear our core code from Flask specific stuff. I would recommend creating a new class ConnexionHeader that wraps this implementation. The ConnexionResponse class can instantiate this class when populating it's attribute self.headers. So whoever is going to implement a new framework support for Connexion will know they have to also implement a method called getlist (or maybe get_list?).
There was a problem hiding this comment.
This is werkzeug.datastructures.Headers, not flask.
There was a problem hiding this comment.
This is werkzeug, not flask.
There was a problem hiding this comment.
The response object here isn't even of type ConnexionResponse. I think this is somewhat premature.
Nevertheless, #464 is up for your review.
There was a problem hiding this comment.
It is a ConnexionResponse instance. Not sure what you meant.
There was a problem hiding this comment.
No, it's a flask.Response object.
Nevertheless, I've updated #464 as you suggested.
According to http://swagger.io/specification/#header-object-68, the headers should be validated. This adds a simple test for incorrect type (header returns a string where an integer is expected), and positive and negative tests against string length.
What feedback do you need from me? |
|
Closing this since it's based on outdated code. Happy to review an updated PR on this. |
Changes proposed in this pull request:
According to http://swagger.io/specification/#header-object-68, the headers should be validated. This adds a simple test for incorrect type (header returns a string where an integer is expected), and positive and negative tests against string length.