Skip to content

Commit 04028bf

Browse files
Erik Poortfacebook-github-bot
Erik Poort
authored andcommittedMay 25, 2018
This ensures no illegal cookies are send to okhttp
Summary: When a website in a ReactNative WebView sets a cookie with an illegal character, this cookie will automatically be added to any request to the same domain. This happens through: BridgeInterceptor.java (l.84) ReactCookieJarContainer.java (l.44) JavaNetCookieJar.java (l.59) ForwardingCookieHandler.java (l.57) ForwardingCookieHandler.java (l.168) CookieManager.java (l.39) The BridgeInterceptor.java then tries to set a Cookie header, which validates both keys and values, and then crashes. okhttp3.6.0 Headers.java (l.320) This fix will strip illegal characters from any cookie that is being passed to the okhttp request. To demonstrate how to crash the app, you can find an example app here: https://github.com/erikpoort/react-native-test-illegal-cookie Or you can load the following url into a webview: https://invalidcookietest.us.dev.monkapps.com/ Press the 'Set cookie' button. Then try to fetch the same url. [ANDROID] [BREAKING] [ReactCookieJarContainer.java] - I'm filtering cookies containing illegal characters from any request. Closes #18203 Differential Revision: D8164302 Pulled By: hramos fbshipit-source-id: 6e58461df594eb2c7aad4c7ad70b76d12ac09b84
1 parent 41975f7 commit 04028bf

File tree

2 files changed

+98
-1
lines changed

2 files changed

+98
-1
lines changed
 

‎ReactAndroid/src/main/java/com/facebook/react/modules/network/ReactCookieJarContainer.java

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package com.facebook.react.modules.network;
22

3+
import java.util.ArrayList;
34
import java.util.Collections;
45
import java.util.List;
56

67
import javax.annotation.Nullable;
78

89
import okhttp3.Cookie;
910
import okhttp3.CookieJar;
11+
import okhttp3.Headers;
1012
import okhttp3.HttpUrl;
1113

1214
/**
@@ -37,7 +39,17 @@ public void saveFromResponse(HttpUrl url, List<Cookie> cookies) {
3739
@Override
3840
public List<Cookie> loadForRequest(HttpUrl url) {
3941
if (cookieJar != null) {
40-
return cookieJar.loadForRequest(url);
42+
List<Cookie> cookies = cookieJar.loadForRequest(url);
43+
ArrayList<Cookie> validatedCookies = new ArrayList<>();
44+
for (Cookie cookie : cookies) {
45+
try {
46+
Headers.Builder cookieChecker = new Headers.Builder();
47+
cookieChecker.add(cookie.name(), cookie.value());
48+
validatedCookies.add(cookie);
49+
} catch (IllegalArgumentException ignored) {
50+
}
51+
}
52+
return validatedCookies;
4153
}
4254
return Collections.emptyList();
4355
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Copyright (c) 2015-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.modules.network;
9+
10+
import com.facebook.react.modules.network.ReactCookieJarContainer;
11+
import okhttp3.Cookie;
12+
import okhttp3.CookieJar;
13+
import okhttp3.HttpUrl;
14+
import java.util.List;
15+
import java.util.ArrayList;
16+
17+
import org.junit.Test;
18+
import org.junit.runner.RunWith;
19+
import org.powermock.core.classloader.annotations.PowerMockIgnore;
20+
import org.powermock.core.classloader.annotations.PrepareForTest;
21+
import org.robolectric.RobolectricTestRunner;
22+
23+
import static org.fest.assertions.api.Assertions.assertThat;
24+
import static org.mockito.Mockito.any;
25+
import static org.mockito.Mockito.mock;
26+
import static org.mockito.Mockito.verify;
27+
import static org.mockito.Mockito.when;
28+
29+
/**
30+
* Tests for {@link NetworkingModule}.
31+
*/
32+
@PrepareForTest({
33+
ReactCookieJarContainer.class
34+
})
35+
@RunWith(RobolectricTestRunner.class)
36+
@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"})
37+
38+
public class ReactCookieJarContainerTest {
39+
40+
@Test
41+
public void testMissingJar() throws Exception {
42+
ReactCookieJarContainer jarContainer = mock(ReactCookieJarContainer.class);
43+
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
44+
}
45+
46+
@Test
47+
public void testEmptyCookies() throws Exception {
48+
ReactCookieJarContainer jarContainer = mock(ReactCookieJarContainer.class);
49+
List<Cookie> cookies = new ArrayList<>();
50+
when(jarContainer.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
51+
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
52+
}
53+
54+
@Test
55+
public void testValidCookies() throws Exception {
56+
ReactCookieJarContainer jarContainer = new ReactCookieJarContainer();
57+
CookieJar cookieJar = mock(CookieJar.class);
58+
jarContainer.setCookieJar(cookieJar);
59+
List<Cookie> cookies = new ArrayList<>();
60+
cookies.add(new Cookie.Builder()
61+
.name("valid")
62+
.value("valid value")
63+
.domain("domain")
64+
.build()
65+
);
66+
when(cookieJar.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
67+
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(1);
68+
}
69+
70+
@Test
71+
public void testInvalidCookies() throws Exception {
72+
ReactCookieJarContainer jarContainer = new ReactCookieJarContainer();
73+
CookieJar cookieJar = mock(CookieJar.class);
74+
jarContainer.setCookieJar(cookieJar);
75+
List<Cookie> cookies = new ArrayList<>();
76+
cookies.add(new Cookie.Builder()
77+
.name("valid")
78+
.value("înválíd välūė")
79+
.domain("domain")
80+
.build()
81+
);
82+
when(cookieJar.loadForRequest(any(HttpUrl.class))).thenReturn(cookies);
83+
assertThat(jarContainer.loadForRequest(any(HttpUrl.class)).size()).isEqualTo(0);
84+
}
85+
}

0 commit comments

Comments
 (0)
Please sign in to comment.