-
-
Notifications
You must be signed in to change notification settings - Fork 175
Fix: Handle HTTP NO CONTENT status code (204) to prevent null reference exceptions in HttpBase.cs #188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
exceptions. NO_CONTENT is not considered as being an error, and therefore previous code would try to parse body data even with a NO_CONTENT being returned. Various backend frameworks use NO_CONTENT as a way to gracefully indicates that a specific resource is empty (for instance, if you try to get the list of items in a player's inventory and the inventory is empty).
@@ -8,6 +8,8 @@ namespace Proyecto26 | |||
{ | |||
public static class HttpBase | |||
{ | |||
public static int HTTP_NO_CONTENT = 204; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be a const
instead of static
. Might be good to scope it private
instead of public
too, since it doesn't seem like anything outside of HttpBase
will need to know about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@L-Naej hello mate, thanks for your contribution! Please check the @StevenGarberg's comment and let us know! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me change that from develop branch, thanks for your help @StevenGarberg! <3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're welcome! Thank you for making such a great library @jdnichollsc. I use it in all of my Unity projects to talk to my web APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@StevenGarberg do you want to be a maintainer of this lib? <3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdnichollsc Absolutely I do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry guys, was completely off from internet for personal reasons, thank you very much for handling this!
Indeed I don't know what I was thinking with these public static, silly mistake...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha no problem, thanks for your help! <3
Fix: Handle HTTP NO CONTENT status code (204) to prevent null reference exceptions in HttpBase.cs
Fix: Handle HTTP NO CONTENT status code (204) to prevent null reference exceptions in HttpBase.cs
NO_CONTENT is not considered as being an error in the HTTP standard, and therefore previous code would try to parse body data even with a NO_CONTENT being returned, leading to a parsing exception.
This is because as NO_CONTENT is not an error, returning an empty Response Body with a 204 status code does not return false for UnityWebRequest.isHttpError which is used in Extensions.IsValidRequest() to know if an exception must be thrown or not.
Various backend frameworks use NO_CONTENT as a way to gracefully indicates that a specific resource is empty (for instance, if you try to get the list of items in a player's inventory and the inventory is empty).
This PR just checks for the status code before trying to parse the response body in HttpBase.cs.
It is a very lightweight fix and should not break projects using the library because it's due to how UnityWebRequest handles HTTP codes (UnityWebRequest.isHttpError won't return true for a 204 NO_CONTENT).