Skip to content

Fixed bug in ParseOkHttpClient which didn't handle ParseFile File uploads properly. #364

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fixed bug in ParseOkHttpClient which didn't handle ParseFile File uploads properly. #364

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad1a74a
Fixed bug in ParseOkHttpClient which didn't handle ParseFile File upl…
mgarnerdev Jan 22, 2016
fc5d60b
Merge pull request #1 from michaelgarnerdev/michaelgarnerdev-patch-1
mgarnerdev Jan 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions Parse/src/main/java/com/parse/ParseOkHttpClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,7 @@ public ParseOkHttpClient(int socketOperationTimeout, SSLSessionCache sslSessionC
// Set Body
ParseHttpBody parseBody = parseRequest.getBody();
ParseOkHttpRequestBody okHttpRequestBody = null;
if(parseBody instanceof ParseByteArrayHttpBody) {
okHttpRequestBody = new ParseOkHttpRequestBody(parseBody);
}
okHttpRequestBody = new ParseOkHttpRequestBody(parseBody);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will cause NPE if parseBody is null when actually executing the request since all the methods on `ParseOkHttpRequestBody use it: https://github.com/ParsePlatform/Parse-SDK-Android/blob/master/Parse/src/main/java/com/parse/ParseOkHttpClient.java#L281

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, I do not think it will cause NPE sicne we only use okHttpRequestBody for POST and PUT request. In that situation parseBody should not be null.
If we add the null checking, when the POST or PUT has a null body, okHttpRequestBuilder will throw IllegalArgumentException.
If we do not add the null checking, when the POST or PUT has a null body, the exception will be thrown when we actually write the body.
In this situation, I think probably it is better to add the null checking. Let's prevent the error as early as possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelgarnerdev Mind adding the null check?

switch (method) {
case PUT:
okHttpRequestBuilder.put(okHttpRequestBody);
Expand Down