pyopenapi/pyswagger

Headers in Response should be case insensitives

Kyria opened this issue · 6 comments

Kyria commented

From RFC 2616 - "Hypertext Transfer Protocol -- HTTP/1.1", Section 4.2, "Message Headers" and RFC 7230:

Each header field consists of a case-insensitive field name followed
by a colon (":"), optional leading whitespace, the field value, and
optional trailing whitespace.

Following this statement, can you make the required change in the Response object ? (you could base that change on https://github.com/requests/requests/blob/master/requests/structures.py#L15 for example)

Reason behind this change
HTTP Servers (nginx, apache, whatever) have to follow these RFC, that means they can send you headers capitalized (Content-Type for example), but also without upper case at all (content-type)

In this case, when you work with headers, adding capitalized version in your check, if the server once send the lower case version, it'll fail because Response object is case sensitive for headers

Example:

res = client.request(some_operation())

# here we check header for the "Expires" header to manage caching
# but if the server returns "expires", as it could happen, we won't do anything
if res.header['Expires']:
    # do some caching stuff

Thanks !

Thanks for reporting this issue, I'll fix it in next release.

@Kyria I just make a new dict based on requests (the reference implementation you mentioned), changes are here (85c2746)

If you are capable to pull it and see if your usage on original dict is broken when applying this change, please feel free to let me know. If you don't have time to do it, I'll still make a release this weekend.

Kyria commented

@mission-liao sorry for the late reply...

The Request part seems to work just as before :) (I mean, it works fine, and case insensitive dict is working)
It looks like you made the change on Request and forgot Response actually, so currently it's not fully working (as intended from my first post)

After, it's shouldn't be causing any issue (in my usage at least), but i'll test it as soon as the change will be present.

@Kyria cool, I will find some time to fix it and release this weekend.

@Kyria Just release v0.8.38 to fix this issue, please feel free to let me know if anything goes wrong.

Kyria commented

https://github.com/pyopenapi/pyswagger/blob/develop/pyswagger/io.py#L360

I think you just forgot this one too, which means that as soon as we do a reset() to reuse the app.op['xxx']() tuple in the client, it's not anymore case insensitive.

Anyway, thanks for your work :) !