Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Fixed status code 204/205 handling #3292

Merged
merged 1 commit into from Mar 14, 2016
Merged

Conversation

kcharwood
Copy link
Contributor

If a server returned a 204/205 with no response content type and no response data, the response serializer would fail. This patch allows response serialization to continue without error if no response content type header is defined, and there are zero bytes of response of data.

If the server returns a response content type header OR response data, the serializer will respect the old serialization logic. This means that if a response content type is listed in the header, we are expecting some type of data response.

This patches #2797

If a server returned a 205 with no response content type and no response data, the response serializer would fail. This patch allows response serialization to continue without error if no response content type header is defined, and there are zero bytes of response of data.

If the server returns a response content type header OR response data, the serializar will respect the old serialization logic. This means that if a response content type is listed in the header, we are expecting some type of data response.
@kcharwood kcharwood changed the title Fixed status code 205 handling Fixed status code 204/205 handling Jan 14, 2016
@codecov-io
Copy link

Current coverage is 74.20%

Merging #3292 into master will decrease coverage by -0.25% as of f1c6025

@@            master   #3292   diff @@
======================================
  Files            5       5       
  Stmts         3002    3004     +2
  Branches         0       0       
  Methods                          
======================================
- Hit           2235    2229     -6
  Partial          0       0       
- Missed         767     775     +8

Review entire Coverage Diff as of f1c6025

Powered by Codecov. Updated on successful CI builds.

@kcharwood
Copy link
Contributor Author

cc @kylef @cnoon @jshier @0xced

Any thoughts here on this patch?

@cnoon
Copy link
Member

cnoon commented Feb 4, 2016

Hey @kcharwood, sorry for not getting back to you sooner.

It's a bit difficult for me to tell exactly what all the moving pieces are here in this logic. I made a very similar type change to Alamofire about 4 months ago to support 204 and 205 cases properly. I stopped validating the content type if there was no data returned which you can see here. The reason was that iOS will "infer" a content-type even if one isn't returned by the server. I was seeing "text/plain" being reported as the MIMEType even though nothing was returned. Because of this behavior, the only consistent way around the problem that I could find was to just ignore the content type validation if no data was returned. That made things simple and seemed like a reasonable tradeoff.

I can't tell if that's exactly what you're doing here, but that ended up being my reasoning.

@kcharwood
Copy link
Contributor Author

My check here is adding an additional check to prevent response type validation if there is no MIME type and the response data length == 0.

The inferred content type however may throw a wrench in that?

kcharwood added a commit that referenced this pull request Mar 14, 2016
@kcharwood kcharwood merged commit 3e8addb into master Mar 14, 2016
@kcharwood kcharwood deleted the bug/properly_handle_205 branch March 31, 2016 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants