From 5cded452e0eea9902fde99f7b2528b63ec0bc695 Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Tue, 7 Feb 2017 18:36:55 -0800 Subject: [PATCH] Breaking Change: Android: Support withCredentials flag in XHRs Corresponding iOS PR: #12275 Respect the withCredentials XMLHttpRequest flag for sending cookies with requests. This can reduce payload sizes where large cookies are set for domains. This should fix #5347. This is a breaking change because it alters the default behavior of XHR. Prior to this change, XHR would send cookies by default. After this change, by default, XHR does not send cookies which is consistent with the default behavior of XHR on web for cross-site requests. Developers can restore the previous behavior by passing `true` for XHR's `withCredentials` argument. **Test plan (required)** Verified in a test app that XHR works properly when specifying `withCredentials` as `true`, `false`, and `undefined`. Also, my team uses this change in our app. Adam Comella Microsoft Corp. --- Libraries/Network/RCTNetworking.android.js | 6 ++-- Libraries/Network/XMLHttpRequest.js | 2 ++ .../modules/network/NetworkingModule.java | 8 ++++- .../modules/network/NetworkingModuleTest.java | 30 ++++++++++++------- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/Libraries/Network/RCTNetworking.android.js b/Libraries/Network/RCTNetworking.android.js index e6c879bc4e80ab..e8bee18a37a4a5 100644 --- a/Libraries/Network/RCTNetworking.android.js +++ b/Libraries/Network/RCTNetworking.android.js @@ -56,7 +56,8 @@ class RCTNetworking extends NativeEventEmitter { responseType: 'text' | 'base64', incrementalUpdates: boolean, timeout: number, - callback: (requestId: number) => any + callback: (requestId: number) => any, + withCredentials: boolean ) { const body = convertRequestBody(data); if (body && body.formData) { @@ -74,7 +75,8 @@ class RCTNetworking extends NativeEventEmitter { {...body, trackingName}, responseType, incrementalUpdates, - timeout + timeout, + withCredentials ); callback(requestId); } diff --git a/Libraries/Network/XMLHttpRequest.js b/Libraries/Network/XMLHttpRequest.js index bf6d30191a2e33..85f80e63cd1acc 100644 --- a/Libraries/Network/XMLHttpRequest.js +++ b/Libraries/Network/XMLHttpRequest.js @@ -117,6 +117,7 @@ class XMLHttpRequest extends EventTarget(...XHR_EVENTS) { status: number = 0; timeout: number = 0; responseURL: ?string; + withCredentials: boolean = false upload: XMLHttpRequestEventTarget = new XMLHttpRequestEventTarget(); @@ -499,6 +500,7 @@ class XMLHttpRequest extends EventTarget(...XHR_EVENTS) { incrementalEvents, this.timeout, this.__didCreateRequest.bind(this), + this.withCredentials ); } 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 d5396c1840a098..dce9dc31193f4d 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 @@ -36,6 +36,7 @@ import okhttp3.Call; import okhttp3.Callback; +import okhttp3.CookieJar; import okhttp3.Headers; import okhttp3.Interceptor; import okhttp3.JavaNetCookieJar; @@ -167,7 +168,8 @@ public void sendRequest( ReadableMap data, final String responseType, final boolean useIncrementalUpdates, - int timeout) { + int timeout, + boolean withCredentials) { Request.Builder requestBuilder = new Request.Builder().url(url); if (requestId != 0) { @@ -177,6 +179,10 @@ public void sendRequest( final RCTDeviceEventEmitter eventEmitter = getEventEmitter(executorToken); OkHttpClient.Builder clientBuilder = mClient.newBuilder(); + if (!withCredentials) { + clientBuilder.cookieJar(CookieJar.NO_COOKIES); + } + // If JS is listening for progress updates, install a ProgressResponseBody that intercepts the // response and counts bytes received. if (useIncrementalUpdates) { diff --git a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java index 8849002c068e16..fc9b5adb926c05 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/modules/network/NetworkingModuleTest.java @@ -100,7 +100,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { /* body */ null, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); @@ -135,7 +136,8 @@ public void testFailGetWithInvalidHeadersStruct() throws Exception { /* body */ null, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); verifyErrorEmit(emitter, 0); } @@ -166,7 +168,8 @@ public void testFailPostWithoutContentType() throws Exception { body, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); verifyErrorEmit(emitter, 0); } @@ -227,7 +230,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { body, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); @@ -270,7 +274,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { null, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); verify(httpClient).newCall(argumentCaptor.capture()); Headers requestHeaders = argumentCaptor.getValue().headers(); @@ -324,7 +329,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { body, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); // verify url, method, headers ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); @@ -389,7 +395,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { body, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); // verify url, method, headers ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Request.class); @@ -492,7 +499,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { body, /* responseType */ "text", /* useIncrementalUpdates*/ true, - /* timeout */ 0); + /* timeout */ 0, + /* withCredentials */ false); // verify RequestBodyPart for image PowerMockito.verifyStatic(times(1)); @@ -556,7 +564,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { null, /* responseType */ "text", /* useIncrementalUpdates*/ true, - 0); + /* timeout */ 0, + /* withCredentials */ false); } verify(httpClient, times(3)).newCall(any(Request.class)); @@ -606,7 +615,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { null, /* responseType */ "text", /* useIncrementalUpdates*/ true, - 0); + /* timeout */ 0, + /* withCredentials */ false); } verify(httpClient, times(3)).newCall(any(Request.class));