diff --git a/Examples/UIExplorer/XHRExample.android.js b/Examples/UIExplorer/XHRExample.android.js index ab272d133..3696a2bfb 100644 --- a/Examples/UIExplorer/XHRExample.android.js +++ b/Examples/UIExplorer/XHRExample.android.js @@ -18,7 +18,6 @@ var React = require('react-native'); var { PixelRatio, - ProgressBarAndroid, StyleSheet, Text, TextInput, @@ -62,6 +61,7 @@ class Downloader extends React.Component { this.setState({ downloaded: xhr.responseText.length, }); + console.log(xhr.responseText.length); } else if (xhr.readyState === xhr.DONE) { if (this.cancelled) { this.cancelled = false; @@ -83,8 +83,6 @@ class Downloader extends React.Component { } }; xhr.open('GET', 'http://www.gutenberg.org/cache/epub/100/pg100.txt'); - // Avoid gzip so we can actually show progress - xhr.setRequestHeader('Accept-Encoding', ''); xhr.send(); this.xhr = xhr; @@ -116,8 +114,6 @@ class Downloader extends React.Component { return ( {button} - {this.state.status} ); diff --git a/Libraries/Network/RCTNetworking.android.js b/Libraries/Network/RCTNetworking.android.js index e8c4be0bd..8d21d8133 100644 --- a/Libraries/Network/RCTNetworking.android.js +++ b/Libraries/Network/RCTNetworking.android.js @@ -25,7 +25,7 @@ var generateRequestId = function() { */ class RCTNetworking { - static sendRequest(method, url, headers, data, useIncrementalUpdates) { + static sendRequest(method, url, headers, data, callback) { var requestId = generateRequestId(); RCTNetworkingNative.sendRequest( method, @@ -33,7 +33,7 @@ class RCTNetworking { requestId, headers, data, - useIncrementalUpdates); + callback); return requestId; } diff --git a/Libraries/Network/RCTNetworking.ios.js b/Libraries/Network/RCTNetworking.ios.js deleted file mode 100644 index 75e85b4fd..000000000 --- a/Libraries/Network/RCTNetworking.ios.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - * @providesModule RCTNetworking - */ -'use strict'; - -module.exports = require('NativeModules').Networking; diff --git a/Libraries/Network/XMLHttpRequest.android.js b/Libraries/Network/XMLHttpRequest.android.js index e0ebc77d0..ba7a84ad9 100644 --- a/Libraries/Network/XMLHttpRequest.android.js +++ b/Libraries/Network/XMLHttpRequest.android.js @@ -26,6 +26,14 @@ function convertHeadersMapToArray(headers: Object): Array
{ } class XMLHttpRequest extends XMLHttpRequestBase { + + _requestId: ?number; + + constructor() { + super(); + this._requestId = null; + } + sendImpl(method: ?string, url: ?string, headers: Object, data: any): void { var body; if (typeof data === 'string') { @@ -41,15 +49,17 @@ class XMLHttpRequest extends XMLHttpRequestBase { body = data; } - var useIncrementalUpdates = this.onreadystatechange ? true : false; - var requestId = RCTNetworking.sendRequest( + this._requestId = RCTNetworking.sendRequest( method, url, convertHeadersMapToArray(headers), body, - useIncrementalUpdates + this.callback.bind(this) ); - this.didCreateRequest(requestId); + } + + abortImpl(): void { + this._requestId && RCTNetworking.abortRequest(this._requestId); } } diff --git a/Libraries/Network/XMLHttpRequest.ios.js b/Libraries/Network/XMLHttpRequest.ios.js index 56b319a25..29b5597ad 100644 --- a/Libraries/Network/XMLHttpRequest.ios.js +++ b/Libraries/Network/XMLHttpRequest.ios.js @@ -12,18 +12,95 @@ 'use strict'; var FormData = require('FormData'); -var RCTNetworking = require('RCTNetworking'); +var RCTNetworking = require('NativeModules').Networking; var RCTDeviceEventEmitter = require('RCTDeviceEventEmitter'); var XMLHttpRequestBase = require('XMLHttpRequestBase'); class XMLHttpRequest extends XMLHttpRequestBase { + + _requestId: ?number; + _subscriptions: [any]; + upload: { + onprogress?: (event: Object) => void; + }; + constructor() { super(); - // iOS supports upload + this._requestId = null; + this._subscriptions = []; this.upload = {}; } + _didCreateRequest(requestId: number): void { + this._requestId = requestId; + this._subscriptions.push(RCTDeviceEventEmitter.addListener( + 'didSendNetworkData', + (args) => this._didUploadProgress.call(this, args[0], args[1], args[2]) + )); + this._subscriptions.push(RCTDeviceEventEmitter.addListener( + 'didReceiveNetworkResponse', + (args) => this._didReceiveResponse.call(this, args[0], args[1], args[2]) + )); + this._subscriptions.push(RCTDeviceEventEmitter.addListener( + 'didReceiveNetworkData', + (args) => this._didReceiveData.call(this, args[0], args[1]) + )); + this._subscriptions.push(RCTDeviceEventEmitter.addListener( + 'didCompleteNetworkResponse', + (args) => this._didCompleteResponse.call(this, args[0], args[1]) + )); + } + + _didUploadProgress(requestId: number, progress: number, total: number): void { + if (requestId === this._requestId && this.upload.onprogress) { + var event = { + lengthComputable: true, + loaded: progress, + total, + }; + this.upload.onprogress(event); + } + } + + _didReceiveResponse(requestId: number, status: number, responseHeaders: ?Object): void { + if (requestId === this._requestId) { + this.status = status; + this.setResponseHeaders(responseHeaders); + this.setReadyState(this.HEADERS_RECEIVED); + } + } + + _didReceiveData(requestId: number, responseText: string): void { + if (requestId === this._requestId) { + if (!this.responseText) { + this.responseText = responseText; + } else { + this.responseText += responseText; + } + this.setReadyState(this.LOADING); + } + } + + _didCompleteResponse(requestId: number, error: string): void { + if (requestId === this._requestId) { + if (error) { + this.responseText = error; + } + this._clearSubscriptions(); + this._requestId = null; + this.setReadyState(this.DONE); + } + } + + _clearSubscriptions(): void { + for (var i = 0; i < this._subscriptions.length; i++) { + var sub = this._subscriptions[i]; + sub.remove(); + } + this._subscriptions = []; + } + sendImpl(method: ?string, url: ?string, headers: Object, data: any): void { if (typeof data === 'string') { data = {string: data}; @@ -38,9 +115,17 @@ class XMLHttpRequest extends XMLHttpRequestBase { headers, incrementalUpdates: this.onreadystatechange ? true : false, }, - this.didCreateRequest.bind(this) + this._didCreateRequest.bind(this) ); } + + abortImpl(): void { + if (this._requestId) { + RCTNetworking.cancelRequest(this._requestId); + this._clearSubscriptions(); + this._requestId = null; + } + } } module.exports = XMLHttpRequest; diff --git a/Libraries/Network/XMLHttpRequestBase.js b/Libraries/Network/XMLHttpRequestBase.js index e37bf6621..0621589e8 100644 --- a/Libraries/Network/XMLHttpRequestBase.js +++ b/Libraries/Network/XMLHttpRequestBase.js @@ -11,9 +11,6 @@ */ 'use strict'; -var RCTNetworking = require('RCTNetworking'); -var RCTDeviceEventEmitter = require('RCTDeviceEventEmitter'); - /** * Shared base for platform-specific XMLHttpRequest implementations. */ @@ -33,13 +30,6 @@ class XMLHttpRequestBase { responseText: ?string; status: number; - upload: ?{ - onprogress?: (event: Object) => void; - }; - - _requestId: ?number; - _subscriptions: [any]; - _method: ?string; _url: ?string; _headers: Object; @@ -70,81 +60,9 @@ class XMLHttpRequestBase { this.responseText = ''; this.status = 0; - this._requestId = null; - this._headers = {}; this._sent = false; this._lowerCaseResponseHeaders = {}; - - this._clearSubscriptions(); - } - - didCreateRequest(requestId: number): void { - this._requestId = requestId; - this._subscriptions.push(RCTDeviceEventEmitter.addListener( - 'didSendNetworkData', - (args) => this._didUploadProgress.call(this, ...args) - )); - this._subscriptions.push(RCTDeviceEventEmitter.addListener( - 'didReceiveNetworkResponse', - (args) => this._didReceiveResponse.call(this, ...args) - )); - this._subscriptions.push(RCTDeviceEventEmitter.addListener( - 'didReceiveNetworkData', - (args) => this._didReceiveData.call(this, ...args) - )); - this._subscriptions.push(RCTDeviceEventEmitter.addListener( - 'didCompleteNetworkResponse', - (args) => this._didCompleteResponse.call(this, ...args) - )); - } - - _didUploadProgress(requestId: number, progress: number, total: number): void { - if (requestId === this._requestId && this.upload && this.upload.onprogress) { - var event = { - lengthComputable: true, - loaded: progress, - total, - }; - this.upload.onprogress(event); - } - } - - _didReceiveResponse(requestId: number, status: number, responseHeaders: ?Object): void { - if (requestId === this._requestId) { - this.status = status; - this.setResponseHeaders(responseHeaders); - this.setReadyState(this.HEADERS_RECEIVED); - } - } - - _didReceiveData(requestId: number, responseText: string): void { - if (requestId === this._requestId) { - if (!this.responseText) { - this.responseText = responseText; - } else { - this.responseText += responseText; - } - this.setReadyState(this.LOADING); - } - } - - _didCompleteResponse(requestId: number, error: string): void { - if (requestId === this._requestId) { - if (error) { - this.responseText = error; - } - this._clearSubscriptions(); - this._requestId = null; - this.setReadyState(this.DONE); - } - } - - _clearSubscriptions(): void { - (this._subscriptions || []).forEach(sub => { - sub.remove(); - }); - this._subscriptions = []; } getAllResponseHeaders(): ?string { @@ -190,6 +108,10 @@ class XMLHttpRequestBase { throw new Error('Subclass must define sendImpl method'); } + abortImpl(): void { + throw new Error('Subclass must define abortImpl method'); + } + send(data: any): void { if (this.readyState !== this.OPENED) { throw new Error('Request has not been opened'); @@ -203,10 +125,7 @@ class XMLHttpRequestBase { abort(): void { this._aborted = true; - if (this._requestId) { - console.log('calling abort', this._requestId); - RCTNetworking.abortRequest(this._requestId); - } + this.abortImpl(); // only call onreadystatechange if there is something to abort, // below logic is per spec if (!(this.readyState === this.UNSENT || @@ -219,6 +138,16 @@ class XMLHttpRequestBase { this._reset(); } + callback(status: number, responseHeaders: ?Object, responseText: string): void { + if (this._aborted) { + return; + } + this.status = status; + this.setResponseHeaders(responseHeaders || {}); + this.responseText = responseText; + this.setReadyState(this.DONE); + } + setResponseHeaders(responseHeaders: ?Object): void { this.responseHeaders = responseHeaders || null; var headers = responseHeaders || {}; diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java index 39d2cb810..3ebf4cd06 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java @@ -13,20 +13,17 @@ import javax.annotation.Nullable; import java.io.IOException; import java.io.InputStream; -import java.io.Reader; import com.facebook.react.bridge.Arguments; +import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.GuardedAsyncTask; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; -import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; -import com.facebook.react.modules.core.DeviceEventManagerModule; -import com.squareup.okhttp.Callback; import com.squareup.okhttp.Headers; import com.squareup.okhttp.MediaType; import com.squareup.okhttp.MultipartBuilder; @@ -34,9 +31,6 @@ import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Request; import com.squareup.okhttp.RequestBody; import com.squareup.okhttp.Response; -import com.squareup.okhttp.ResponseBody; - -import static java.lang.Math.min; /** * Implements the XMLHttpRequest JavaScript interface. @@ -50,11 +44,6 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { private static final String REQUEST_BODY_KEY_FORMDATA = "formData"; private static final String USER_AGENT_HEADER_NAME = "user-agent"; - private static final int MIN_BUFFER_SIZE = 8 * 1024; // 8kb - private static final int MAX_BUFFER_SIZE = 512 * 1024; // 512kb - - private static final int CHUNK_TIMEOUT_NS = 100 * 1000000; // 100ms - private final OkHttpClient mClient; private final @Nullable String mDefaultUserAgent; private boolean mShuttingDown; @@ -104,10 +93,15 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { public void sendRequest( String method, String url, - final int requestId, + int requestId, ReadableArray headers, ReadableMap data, - final boolean useIncrementalUpdates) { + final Callback callback) { + // We need to call the callback to avoid leaking memory on JS even when input for sending + // request is erroneous or insufficient. For non-http based failures we use code 0, which is + // interpreted as a transport error. + // Callback accepts following arguments: responseCode, headersString, responseBody + Request.Builder requestBuilder = new Request.Builder().url(url); if (requestId != 0) { @@ -116,7 +110,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { Headers requestHeaders = extractHeaders(headers, data); if (requestHeaders == null) { - onRequestError(requestId, "Unrecognized headers format"); + callback.invoke(0, null, "Unrecognized headers format"); return; } String contentType = requestHeaders.get(CONTENT_TYPE_HEADER_NAME); @@ -127,7 +121,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { requestBuilder.method(method, null); } else if (data.hasKey(REQUEST_BODY_KEY_STRING)) { if (contentType == null) { - onRequestError(requestId, "Payload is set but no content-type header specified"); + callback.invoke(0, null, "Payload is set but no content-type header specified"); return; } String body = data.getString(REQUEST_BODY_KEY_STRING); @@ -135,7 +129,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { if (RequestBodyUtil.isGzipEncoding(contentEncoding)) { RequestBody requestBody = RequestBodyUtil.createGzip(contentMediaType, body); if (requestBody == null) { - onRequestError(requestId, "Failed to gzip request body"); + callback.invoke(0, null, "Failed to gzip request body"); return; } requestBuilder.method(method, requestBody); @@ -144,14 +138,14 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { } } else if (data.hasKey(REQUEST_BODY_KEY_URI)) { if (contentType == null) { - onRequestError(requestId, "Payload is set but no content-type header specified"); + callback.invoke(0, null, "Payload is set but no content-type header specified"); return; } String uri = data.getString(REQUEST_BODY_KEY_URI); InputStream fileInputStream = RequestBodyUtil.getFileInputStream(getReactApplicationContext(), uri); if (fileInputStream == null) { - onRequestError(requestId, "Could not retrieve file for uri " + uri); + callback.invoke(0, null, "Could not retrieve file for uri " + uri); return; } requestBuilder.method( @@ -162,7 +156,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { contentType = "multipart/form-data"; } ReadableArray parts = data.getArray(REQUEST_BODY_KEY_FORMDATA); - MultipartBuilder multipartBuilder = constructMultipartBody(parts, contentType, requestId); + MultipartBuilder multipartBuilder = constructMultipartBody(parts, contentType, callback); if (multipartBuilder == null) { return; } @@ -174,13 +168,16 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { } mClient.newCall(requestBuilder.build()).enqueue( - new Callback() { + new com.squareup.okhttp.Callback() { @Override public void onFailure(Request request, IOException e) { if (mShuttingDown) { return; } - onRequestError(requestId, e.getMessage()); + // We need to call the callback to avoid leaking memory on JS even when input for + // sending request is erronous or insufficient. For non-http based failures we use + // code 0, which is interpreted as a transport error + callback.invoke(0, null, e.getMessage()); } @Override @@ -188,115 +185,34 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { if (mShuttingDown) { return; } - - // Before we touch the body send headers to JS - onResponseReceived(requestId, response); - - ResponseBody responseBody = response.body(); + String responseBody; try { - if (useIncrementalUpdates) { - readWithProgress(requestId, responseBody); - onRequestSuccess(requestId); - } else { - onDataReceived(requestId, responseBody.string()); - onRequestSuccess(requestId); - } + responseBody = response.body().string(); } catch (IOException e) { - onRequestError(requestId, e.getMessage()); + // The stream has been cancelled or closed, nothing we can do + callback.invoke(0, null, e.getMessage()); + return; } + + WritableMap responseHeaders = Arguments.createMap(); + Headers headers = response.headers(); + for (int i = 0; i < headers.size(); i++) { + String headerName = headers.name(i); + // multiple values for the same header + if (responseHeaders.hasKey(headerName)) { + responseHeaders.putString( + headerName, + responseHeaders.getString(headerName) + ", " + headers.value(i)); + } else { + responseHeaders.putString(headerName, headers.value(i)); + } + } + + callback.invoke(response.code(), responseHeaders, responseBody); } }); } - private void readWithProgress(int requestId, ResponseBody responseBody) throws IOException { - Reader reader = responseBody.charStream(); - try { - StringBuilder sb = new StringBuilder(getBufferSize(responseBody)); - char[] buffer = new char[MIN_BUFFER_SIZE]; - int read; - long last = System.nanoTime(); - while ((read = reader.read(buffer)) != -1) { - sb.append(buffer, 0, read); - long now = System.nanoTime(); - if (shouldDispatch(now, last)) { - onDataReceived(requestId, sb.toString()); - sb.setLength(0); - last = now; - } - } - - if (sb.length() > 0) { - onDataReceived(requestId, sb.toString()); - } - } finally { - reader.close(); - } - } - - private static boolean shouldDispatch(long now, long last) { - return last + CHUNK_TIMEOUT_NS < now; - } - - private static int getBufferSize(ResponseBody responseBody) throws IOException { - long length = responseBody.contentLength(); - if (length == -1) { - return MIN_BUFFER_SIZE; - } else { - return (int) min(length, MAX_BUFFER_SIZE); - } - } - - private void onDataReceived(int requestId, String data) { - WritableArray args = Arguments.createArray(); - args.pushInt(requestId); - args.pushString(data); - - getEventEmitter().emit("didReceiveNetworkData", args); - } - - private void onRequestError(int requestId, String error) { - WritableArray args = Arguments.createArray(); - args.pushInt(requestId); - args.pushString(error); - - getEventEmitter().emit("didCompleteNetworkResponse", args); - } - - private void onRequestSuccess(int requestId) { - WritableArray args = Arguments.createArray(); - args.pushInt(requestId); - args.pushNull(); - - getEventEmitter().emit("didCompleteNetworkResponse", args); - } - - private void onResponseReceived(int requestId, Response response) { - WritableMap headers = translateHeaders(response.headers()); - - WritableArray args = Arguments.createArray(); - args.pushInt(requestId); - args.pushInt(response.code()); - args.pushMap(headers); - - getEventEmitter().emit("didReceiveNetworkResponse", args); - } - - private static WritableMap translateHeaders(Headers headers) { - WritableMap responseHeaders = Arguments.createMap(); - for (int i = 0; i < headers.size(); i++) { - String headerName = headers.name(i); - // multiple values for the same header - if (responseHeaders.hasKey(headerName)) { - responseHeaders.putString( - headerName, - responseHeaders.getString(headerName) + ", " + headers.value(i)); - } else { - responseHeaders.putString(headerName, headers.value(i)); - } - } - return responseHeaders; - } - @ReactMethod public void abortRequest(final int requestId) { // We have to use AsyncTask since this might trigger a NetworkOnMainThreadException, this is an @@ -312,7 +228,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { private @Nullable MultipartBuilder constructMultipartBody( ReadableArray body, String contentType, - int requestId) { + Callback callback) { MultipartBuilder multipartBuilder = new MultipartBuilder(); multipartBuilder.type(MediaType.parse(contentType)); @@ -323,7 +239,7 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { ReadableArray headersArray = bodyPart.getArray("headers"); Headers headers = extractHeaders(headersArray, null); if (headers == null) { - onRequestError(requestId, "Missing or invalid header format for FormData part."); + callback.invoke(0, null, "Missing or invalid header format for FormData part."); return null; } MediaType partContentType = null; @@ -340,19 +256,19 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { multipartBuilder.addPart(headers, RequestBody.create(partContentType, bodyValue)); } else if (bodyPart.hasKey(REQUEST_BODY_KEY_URI)) { if (partContentType == null) { - onRequestError(requestId, "Binary FormData part needs a content-type header."); + callback.invoke(0, null, "Binary FormData part needs a content-type header."); return null; } String fileContentUriStr = bodyPart.getString(REQUEST_BODY_KEY_URI); InputStream fileInputStream = RequestBodyUtil.getFileInputStream(getReactApplicationContext(), fileContentUriStr); if (fileInputStream == null) { - onRequestError(requestId, "Could not retrieve file for uri " + fileContentUriStr); + callback.invoke(0, null, "Could not retrieve file for uri " + fileContentUriStr); return null; } multipartBuilder.addPart(headers, RequestBodyUtil.create(partContentType, fileInputStream)); } else { - onRequestError(requestId, "Unrecognized FormData part."); + callback.invoke(0, null, "Unrecognized FormData part."); } } return multipartBuilder; @@ -389,9 +305,4 @@ public final class NetworkingModule extends ReactContextBaseJavaModule { return headersBuilder.build(); } - - private DeviceEventManagerModule.RCTDeviceEventEmitter getEventEmitter() { - return getReactApplicationContext() - .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class); - } }