Browse Source

fix(whitelisted origins): Prevent handling of un-whitelisted URLs

* Preventing an unhandled promise rejection when: a URL is loaded by the WebView, but the URL isn't in the origin whitelist, so it is handed off to the OS to handle by calling Linking.openURL(), but Linking.openURL has an error.  The code wasn't catching the error, so this would result in an unhandled promise rejection. Now the error is being caught.

* Fixing a problem where a URL is handled to the OS to deal with, via Linking.openURL, and also loaded in the WebView by making those cases mutually exclusive (they weren't previously).  In more detail: when a URL is loaded by the WebView that isn't in the origin whitelist it is handled off to the OS to handle by calling Linking.openURL.  But, if the onShouldStartLoadWithRequest prop is set, then that function would also be called, and then that would determine whether the URL should be loaded.  This can result in a situation where the URL is passed to Linking.openURL and onShouldStartLoadWithRequest returns true so it is also loaded in the WebView.  The client can fix this by duplicating the origin whitelist logic in their onShouldStartLoadWithRequest of course, but this change makes it so they don't have to.

Co-authored-by: Jason Safaiyeh <safaiyeh@protonmail.com>
aarondail 4 years ago
parent
commit
0442126595
2 changed files with 90 additions and 12 deletions
  1. 10
    4
      src/WebViewShared.tsx
  2. 80
    8
      src/__tests__/WebViewShared-test.js

+ 10
- 4
src/WebViewShared.tsx View File

44
     const { url, lockIdentifier } = nativeEvent;
44
     const { url, lockIdentifier } = nativeEvent;
45
 
45
 
46
     if (!passesWhitelist(compileWhitelist(originWhitelist), url)) {
46
     if (!passesWhitelist(compileWhitelist(originWhitelist), url)) {
47
-      Linking.openURL(url);
47
+      Linking.canOpenURL(url).then((supported) => {
48
+        if (supported) {
49
+          return Linking.openURL(url);
50
+        }
51
+        console.warn(`Can't open url: ${url}`);
52
+        return undefined;
53
+      }).catch(e => {
54
+        console.warn('Error opening URL: ', e);
55
+      });
48
       shouldStart = false;
56
       shouldStart = false;
49
-    }
50
-
51
-    if (onShouldStartLoadWithRequest) {
57
+    } else if (onShouldStartLoadWithRequest) {
52
       shouldStart = onShouldStartLoadWithRequest(nativeEvent);
58
       shouldStart = onShouldStartLoadWithRequest(nativeEvent);
53
     }
59
     }
54
 
60
 

+ 80
- 8
src/__tests__/WebViewShared-test.js View File

5
   createOnShouldStartLoadWithRequest,
5
   createOnShouldStartLoadWithRequest,
6
 } from '../WebViewShared';
6
 } from '../WebViewShared';
7
 
7
 
8
+Linking.openURL.mockResolvedValue(undefined);
9
+Linking.canOpenURL.mockResolvedValue(true);
10
+
11
+// The tests that call createOnShouldStartLoadWithRequest will cause a promise
12
+// to get kicked off (by calling the mocked `Linking.canOpenURL`) that the tests
13
+// _need_ to get run to completion _before_ doing any `expect`ing. The reason
14
+// is: once that promise is resolved another function should get run which will
15
+// call `Linking.openURL`, and we want to test that.
16
+//
17
+// Normally we would probably do something like `await
18
+// createShouldStartLoadWithRequest(...)` in the tests, but that doesn't work
19
+// here because the promise that gets kicked off is not returned (because
20
+// non-test code doesn't need to know about it).
21
+//
22
+// The tests thus need a way to "flush any pending promises" (to make sure
23
+// pending promises run to completion) before doing any `expect`ing. `jest`
24
+// doesn't provide a way to do this out of the box, but we can use this function
25
+// to do it.
26
+//
27
+// See this issue for more discussion: https://github.com/facebook/jest/issues/2157
28
+function flushPromises() {
29
+  return new Promise(resolve => setImmediate(resolve));
30
+}
31
+
32
+
8
 describe('WebViewShared', () => {
33
 describe('WebViewShared', () => {
9
   test('exports defaultOriginWhitelist', () => {
34
   test('exports defaultOriginWhitelist', () => {
10
     expect(defaultOriginWhitelist).toMatchSnapshot();
35
     expect(defaultOriginWhitelist).toMatchSnapshot();
21
 
46
 
22
     const loadRequest = jest.fn();
47
     const loadRequest = jest.fn();
23
 
48
 
24
-    test('loadRequest is called without onShouldStartLoadWithRequest override', () => {
49
+    test('loadRequest is called without onShouldStartLoadWithRequest override', async () => {
25
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
50
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
26
         loadRequest,
51
         loadRequest,
27
         defaultOriginWhitelist,
52
         defaultOriginWhitelist,
28
       );
53
       );
29
 
54
 
30
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
55
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
56
+      
57
+      await flushPromises();
58
+
31
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
59
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
32
       expect(loadRequest).toHaveBeenCalledWith(true, 'https://www.example.com/', 1);
60
       expect(loadRequest).toHaveBeenCalledWith(true, 'https://www.example.com/', 1);
33
     });
61
     });
34
 
62
 
35
-    test('Linking.openURL is called without onShouldStartLoadWithRequest override', () => {
63
+    test('Linking.openURL is called without onShouldStartLoadWithRequest override', async () => {
36
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
64
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
37
         loadRequest,
65
         loadRequest,
38
         defaultOriginWhitelist,
66
         defaultOriginWhitelist,
39
       );
67
       );
40
 
68
 
41
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'invalid://example.com/', lockIdentifier: 2 } });
69
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'invalid://example.com/', lockIdentifier: 2 } });
70
+      
71
+      await flushPromises();
72
+
42
       expect(Linking.openURL).toHaveBeenCalledWith('invalid://example.com/');
73
       expect(Linking.openURL).toHaveBeenCalledWith('invalid://example.com/');
43
       expect(loadRequest).toHaveBeenCalledWith(false, 'invalid://example.com/', 2);
74
       expect(loadRequest).toHaveBeenCalledWith(false, 'invalid://example.com/', 2);
44
     });
75
     });
45
 
76
 
46
-    test('loadRequest with true onShouldStartLoadWithRequest override is called', () => {
77
+    test('loadRequest with true onShouldStartLoadWithRequest override is called', async () => {
47
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
78
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
48
         loadRequest,
79
         loadRequest,
49
         defaultOriginWhitelist,
80
         defaultOriginWhitelist,
51
       );
82
       );
52
 
83
 
53
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
84
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
85
+
86
+      await flushPromises();
87
+
54
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
88
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
55
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'https://www.example.com/', 1);
89
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'https://www.example.com/', 1);
56
     });
90
     });
57
 
91
 
58
-    test('Linking.openURL with true onShouldStartLoadWithRequest override is called for links not passing the whitelist', () => {
92
+    test('Linking.openURL with true onShouldStartLoadWithRequest override is called for links not passing the whitelist', async () => {
59
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
93
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
60
         loadRequest,
94
         loadRequest,
61
         defaultOriginWhitelist,
95
         defaultOriginWhitelist,
62
         alwaysTrueOnShouldStartLoadWithRequest,
96
         alwaysTrueOnShouldStartLoadWithRequest,
63
       );
97
       );
64
 
98
 
99
+      var a = 10;
65
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'invalid://example.com/', lockIdentifier: 1 } });
100
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'invalid://example.com/', lockIdentifier: 1 } });
101
+
102
+      await flushPromises();
103
+
66
       expect(Linking.openURL).toHaveBeenLastCalledWith('invalid://example.com/');
104
       expect(Linking.openURL).toHaveBeenLastCalledWith('invalid://example.com/');
67
-      expect(loadRequest).toHaveBeenLastCalledWith(true, 'invalid://example.com/', 1);
105
+      // We don't expect the URL to have been loaded in the WebView because it
106
+      // is not in the origin whitelist
107
+      expect(loadRequest).toHaveBeenLastCalledWith(false, 'invalid://example.com/', 1);
68
     });
108
     });
69
 
109
 
70
-    test('loadRequest with false onShouldStartLoadWithRequest override is called', () => {
110
+    test('loadRequest with false onShouldStartLoadWithRequest override is called', async () => {
71
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
111
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
72
         loadRequest,
112
         loadRequest,
73
         defaultOriginWhitelist,
113
         defaultOriginWhitelist,
75
       );
115
       );
76
 
116
 
77
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
117
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
118
+
119
+      await flushPromises();
120
+
78
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
121
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
79
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'https://www.example.com/', 1);
122
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'https://www.example.com/', 1);
80
     });
123
     });
81
 
124
 
82
-    test('loadRequest with limited whitelist', () => {
125
+    test('loadRequest with limited whitelist', async () => {
83
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
126
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
84
         loadRequest,
127
         loadRequest,
85
         ['https://*'],
128
         ['https://*'],
86
       );
129
       );
87
 
130
 
88
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
131
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'https://www.example.com/', lockIdentifier: 1 } });
132
+      
133
+      await flushPromises();
134
+
89
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
135
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
90
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'https://www.example.com/', 1);
136
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'https://www.example.com/', 1);
91
 
137
 
92
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'http://insecure.com/', lockIdentifier: 2 } });
138
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'http://insecure.com/', lockIdentifier: 2 } });
139
+
140
+      await flushPromises();
141
+
93
       expect(Linking.openURL).toHaveBeenLastCalledWith('http://insecure.com/');
142
       expect(Linking.openURL).toHaveBeenLastCalledWith('http://insecure.com/');
94
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'http://insecure.com/', 2);
143
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'http://insecure.com/', 2);
95
 
144
 
96
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'git+https://insecure.com/', lockIdentifier: 3 } });
145
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'git+https://insecure.com/', lockIdentifier: 3 } });
146
+      
147
+      await flushPromises();
148
+
97
       expect(Linking.openURL).toHaveBeenLastCalledWith('git+https://insecure.com/');
149
       expect(Linking.openURL).toHaveBeenLastCalledWith('git+https://insecure.com/');
98
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'git+https://insecure.com/', 3);
150
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'git+https://insecure.com/', 3);
99
 
151
 
100
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'fakehttps://insecure.com/', lockIdentifier: 4 } });
152
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'fakehttps://insecure.com/', lockIdentifier: 4 } });
153
+      
154
+      await flushPromises();
155
+
101
       expect(Linking.openURL).toHaveBeenLastCalledWith('fakehttps://insecure.com/');
156
       expect(Linking.openURL).toHaveBeenLastCalledWith('fakehttps://insecure.com/');
102
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'fakehttps://insecure.com/', 4);
157
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'fakehttps://insecure.com/', 4);
103
     });
158
     });
104
 
159
 
105
-    test('loadRequest allows for valid URIs', () => {
160
+    test('loadRequest allows for valid URIs', async () => {
106
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
161
       const onShouldStartLoadWithRequest = createOnShouldStartLoadWithRequest(
107
           loadRequest,
162
           loadRequest,
108
           ['plus+https://*', 'DOT.https://*', 'dash-https://*', '0invalid://*', '+invalid://*'],
163
           ['plus+https://*', 'DOT.https://*', 'dash-https://*', '0invalid://*', '+invalid://*'],
109
       );
164
       );
110
 
165
 
111
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'plus+https://www.example.com/', lockIdentifier: 1 } });
166
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'plus+https://www.example.com/', lockIdentifier: 1 } });
167
+      
168
+      await flushPromises();
112
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
169
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
113
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'plus+https://www.example.com/', 1);
170
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'plus+https://www.example.com/', 1);
114
 
171
 
115
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'DOT.https://www.example.com/', lockIdentifier: 2 } });
172
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'DOT.https://www.example.com/', lockIdentifier: 2 } });
173
+
174
+      await flushPromises();
175
+
116
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
176
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
117
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'DOT.https://www.example.com/', 2);
177
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'DOT.https://www.example.com/', 2);
118
 
178
 
119
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'dash-https://www.example.com/', lockIdentifier: 3 } });
179
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'dash-https://www.example.com/', lockIdentifier: 3 } });
180
+      
181
+      await flushPromises();
182
+
120
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
183
       expect(Linking.openURL).toHaveBeenCalledTimes(0);
121
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'dash-https://www.example.com/', 3);
184
       expect(loadRequest).toHaveBeenLastCalledWith(true, 'dash-https://www.example.com/', 3);
122
 
185
 
123
       onShouldStartLoadWithRequest({ nativeEvent: { url: '0invalid://www.example.com/', lockIdentifier: 4 } });
186
       onShouldStartLoadWithRequest({ nativeEvent: { url: '0invalid://www.example.com/', lockIdentifier: 4 } });
187
+
188
+      await flushPromises();
189
+
124
       expect(Linking.openURL).toHaveBeenLastCalledWith('0invalid://www.example.com/');
190
       expect(Linking.openURL).toHaveBeenLastCalledWith('0invalid://www.example.com/');
125
       expect(loadRequest).toHaveBeenLastCalledWith(false, '0invalid://www.example.com/', 4);
191
       expect(loadRequest).toHaveBeenLastCalledWith(false, '0invalid://www.example.com/', 4);
126
 
192
 
127
       onShouldStartLoadWithRequest({ nativeEvent: { url: '+invalid://www.example.com/', lockIdentifier: 5 } });
193
       onShouldStartLoadWithRequest({ nativeEvent: { url: '+invalid://www.example.com/', lockIdentifier: 5 } });
194
+      
195
+      await flushPromises();
196
+
128
       expect(Linking.openURL).toHaveBeenLastCalledWith('+invalid://www.example.com/');
197
       expect(Linking.openURL).toHaveBeenLastCalledWith('+invalid://www.example.com/');
129
       expect(loadRequest).toHaveBeenLastCalledWith(false, '+invalid://www.example.com/', 5);
198
       expect(loadRequest).toHaveBeenLastCalledWith(false, '+invalid://www.example.com/', 5);
130
 
199
 
131
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'FAKE+plus+https://www.example.com/', lockIdentifier: 6 } });
200
       onShouldStartLoadWithRequest({ nativeEvent: { url: 'FAKE+plus+https://www.example.com/', lockIdentifier: 6 } });
201
+
202
+      await flushPromises();
203
+
132
       expect(Linking.openURL).toHaveBeenLastCalledWith('FAKE+plus+https://www.example.com/');
204
       expect(Linking.openURL).toHaveBeenLastCalledWith('FAKE+plus+https://www.example.com/');
133
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'FAKE+plus+https://www.example.com/', 6);
205
       expect(loadRequest).toHaveBeenLastCalledWith(false, 'FAKE+plus+https://www.example.com/', 6);
134
     });
206
     });