Skip to content

Commit

Permalink
BREAKING: Android: Support withCredentials flag in XHRs
Browse files Browse the repository at this point in the history
Summary:
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.
Closes #12276

Differential Revision: D4673646

Pulled By: ericvicenti

fbshipit-source-id: 924c230c9df72071b3cf9151c3ac201905ac28a5
  • Loading branch information
Adam Comella authored and facebook-github-bot committed Apr 11, 2017
1 parent ecd0df0 commit e544563
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 13 deletions.
6 changes: 4 additions & 2 deletions Libraries/Network/RCTNetworking.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -74,7 +75,8 @@ class RCTNetworking extends NativeEventEmitter {
{...body, trackingName},
responseType,
incrementalUpdates,
timeout
timeout,
withCredentials
);
callback(requestId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import okhttp3.Call;
import okhttp3.Callback;
import okhttp3.CookieJar;
import okhttp3.Headers;
import okhttp3.Interceptor;
import okhttp3.JavaNetCookieJar;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
/* body */ null,
/* responseType */ "text",
/* useIncrementalUpdates*/ true,
/* timeout */ 0);
/* timeout */ 0,
/* withCredentials */ false);

ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
verify(httpClient).newCall(argumentCaptor.capture());
Expand Down Expand Up @@ -135,7 +136,8 @@ public void testFailGetWithInvalidHeadersStruct() throws Exception {
/* body */ null,
/* responseType */ "text",
/* useIncrementalUpdates*/ true,
/* timeout */ 0);
/* timeout */ 0,
/* withCredentials */ false);

verifyErrorEmit(emitter, 0);
}
Expand Down Expand Up @@ -166,7 +168,8 @@ public void testFailPostWithoutContentType() throws Exception {
body,
/* responseType */ "text",
/* useIncrementalUpdates*/ true,
/* timeout */ 0);
/* timeout */ 0,
/* withCredentials */ false);

verifyErrorEmit(emitter, 0);
}
Expand Down Expand Up @@ -227,7 +230,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
body,
/* responseType */ "text",
/* useIncrementalUpdates*/ true,
/* timeout */ 0);
/* timeout */ 0,
/* withCredentials */ false);

ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
verify(httpClient).newCall(argumentCaptor.capture());
Expand Down Expand Up @@ -270,7 +274,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
null,
/* responseType */ "text",
/* useIncrementalUpdates*/ true,
/* timeout */ 0);
/* timeout */ 0,
/* withCredentials */ false);
ArgumentCaptor<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
verify(httpClient).newCall(argumentCaptor.capture());
Headers requestHeaders = argumentCaptor.getValue().headers();
Expand Down Expand Up @@ -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<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
Expand Down Expand Up @@ -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<Request> argumentCaptor = ArgumentCaptor.forClass(Request.class);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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));

Expand Down

0 comments on commit e544563

Please sign in to comment.