Skip to content

Commit

Permalink
Fix blob response parsing for empty body on iOS
Browse files Browse the repository at this point in the history 
Summary:
We currently handle empty body poorly in the iOS blob implementation, this happens because of an early return that cause the blob response to not be processed by the blob module, resulting in an empty string as the body instead of a blob object. We also need to make sure to create an empty blob object when data is nil (empty body) as per the XMLHttpRequest spec. The Android implementation was already handling this properly.

Fixes #18223

Send a HEAD request

```js
fetch('https://apipre.monkimun.com/whoami', {
  body: null,
  method: 'HEAD',
  headers: {
    Accept: 'application/json',
    'Content-Type': 'application/json',
  },
})
```

[IOS][BUGFIX][Blob] - Fix blob response parsing for empty body
Closes #18547

Differential Revision: D7415950

Pulled By: hramos

fbshipit-source-id: 56860532c6171255869f02a0960f55d155184a46
  • Loading branch information
@janicduplessis @facebook-github-bot
janicduplessis authored and facebook-github-bot committed Mar 27, 2018
1 parent de1040f commit f5207ba
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
3 changes: 3 additions & 0 deletions Libraries/Blob/RCTBlobManager.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ - (BOOL)canHandleNetworkingResponse:(NSString *)responseType

- (id)handleNetworkingResponse:(NSURLResponse *)response data:(NSData *)data
{
// An empty body will have nil for data, in this case we need to return
// an empty blob as per the XMLHttpRequest spec.
data = data ?: [NSData new];
return @{
@"blobId": [self store:data],
@"offset": @0,
Expand Down
8 changes: 4 additions & 4 deletions Libraries/Network/RCTNetworking.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,6 @@ - (void)sendData:(NSData *)data
{
RCTAssertThread(_methodQueue, @"sendData: must be called on method queue");

if (data.length == 0) {
return;
}

id responseData = nil;
for (id<RCTNetworkingResponseHandler> handler in _responseHandlers) {
if ([handler canHandleNetworkingResponse:responseType]) {
Expand All @@ -452,6 +448,10 @@ - (void)sendData:(NSData *)data
}

if (!responseData) {
if (data.length == 0) {
return;
}

if ([responseType isEqualToString:@"text"]) {
// No carry storage is required here because the entire data has been loaded.
responseData = [RCTNetworking decodeTextData:data fromResponse:task.response withCarryData:nil];
Expand Down

4 comments on commit f5207ba

@ovr
Copy link
Contributor

@ovr ovr commented on f5207ba Mar 27, 2018

Choose a reason for hiding this comment

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

Cherry-pick into 0.54-stable branch?

@AndrewHenderson
Copy link

Choose a reason for hiding this comment

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

@janicduplessis I've been receiving the "invalid response for Blob" error when uploading a file to a Java Spring backend using FormData.

I copied your changes into my local Libraries and it worked pretty well. I'm no longer receiving an error, but a warning instead.

screen shot 2018-04-06 at 5 34 59 pm

I'm now seeing this as this._response in XMLHttpRequest.js

{
  "size": 0,
  "blobId": "A7CF7562-84AD-4511-AE06-D3F0C939945E",
  "offset": 0,
  "type": "application/json",
  "name": "mobile-receipts"
}

For other successful requests, the size is 2.

@janicduplessis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AndrewHenderson Do you have an example request I could make to reproduce this error?

@AndrewHenderson
Copy link

@AndrewHenderson AndrewHenderson commented on f5207ba Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@janicduplessis Here's my request:

const data = new FormData();

data.append('file', {
  uri: filepath,
  type: 'image/jpg',
  name: 'test_photo.jpg'
});

try {
  const res = await fetch(`${API}/mobile-receipts`, {
    method: 'POST',
    headers: {
      Accept: 'application/json',
      'Content-Type': 'multipart/form-data;',
      Authorization: authToken,
      'upload-date': new Date().toISOString(),
      'customer-id': customerId
    },
    body: data
  });
  console.log(res, res.json())
} catch(error) {
  console.log(error);
}

The endpoint was returning an empty string, which I'm assuming when it tried to parse threw the error.
Once we change the response to a timestamp, things started working properly.

Other endpoints that do not accept multipart/form-data work properly when returning an empty string. It's likely that my use res.json() did it.

Please sign in to comment.