vadymmarkov / Malibu

:surfer: Malibu is a networking library built on promises
https://vadymmarkov.github.io
Other
414 stars 39 forks source link

Remove validation of 204 responses in .validate() #90

Closed OscarApeland closed 6 years ago

OscarApeland commented 6 years ago

Right now, the default .validate() fails when receiving a HTTP 204 NO CONTENT response trough Express, as Express strips away those fields when not needed.

If the maintainers agree, I'd like to remove 204 from the standard .validate() method in ResponseValidation.swift.

Edit: A more detailed explanation of why it doesn't work: If you set application/json as Accept and receive a 204, Express strips the Content-Type field making NSURLRequest do its best to guess the Content-Type itself, and as Express does chunk = '' on a 204, it assumes its text/plain. That obviously fails content type validation.

vadymmarkov commented 6 years ago

@OscarApeland Is it always like that for 204 response or it only happens while using Express?

OscarApeland commented 6 years ago

@vadymmarkov We just use Express, so I don't have any idea about others. I thought that as 204's are NO CONTENT they shouldn't need to have their content validated anyway, just the headers.

JulianNymark commented 6 years ago

from https://tools.ietf.org/html/rfc7231#section-6.3.5, there's this:

A 204 response is terminated by the first empty line after the header fields because it cannot contain a message body.

It doesn't seem to put any restrictions / requirements on the headers, other than:

Metadata in the response header fields refer to the target resource and its selected representation after the requested action was applied.

It seems Express only wipes the headers related to content on the grounds that it makes sense, (as implied by the comment // strip irrelevant headers in the response.js code.)

vadymmarkov commented 6 years ago

Ok, thanks for info @JulianNymark. I think it makes sense to do this change @OscarApeland 👍