Skip to content

Commit 4d9a54a

Browse files
committed
msglist [nfc]: Factor out "keep WebView at a single page" logic
This code has really quite a different character from the rest of what the MessageList component is doing, and even more so from what the rest of the render method is doing. In fact it isn't really even about the message list at all: it's about using a WebView to show a single page, with all navigation handled externally. In its current location, it puts a wide separation between different parts of the actual message-list logic, making it harder to read, understand, or modify. So, make both this code and the actual message-list code easier to read, understand, and modify by separating them out from each other. Also take the opportunity to write some jsdoc explaining what this is supposed to do.
1 parent c0bf408 commit 4d9a54a

File tree

2 files changed

+109
-62
lines changed

2 files changed

+109
-62
lines changed

src/webview/MessageList.js

Lines changed: 8 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,10 @@ import messageListElementHtml from './html/messageListElementHtml';
3333
import generateInboundEvents from './generateInboundEvents';
3434
import { handleWebViewOutboundEvent } from './handleOutboundEvents';
3535
import { base64Utf8Encode } from '../utils/encoding';
36-
import * as logging from '../utils/logging';
37-
import { tryParseUrl } from '../utils/url';
3836
import { caseNarrow, isConversationNarrow } from '../utils/narrow';
3937
import { type BackgroundData, getBackgroundData } from './backgroundData';
4038
import { ensureUnreachable } from '../generics';
39+
import { renderSinglePageWebView } from './SinglePageWebView';
4140

4241
type OuterProps = $ReadOnly<{|
4342
narrow: Narrow,
@@ -196,66 +195,13 @@ class MessageListInner extends React.Component<Props> {
196195
backgroundData.serverEmojiData,
197196
);
198197

199-
// Paranoia^WSecurity: only load `baseUrl`, and only load it once. Any other
200-
// requests should be handed off to the OS, not loaded inside the WebView.
201-
const onShouldStartLoadWithRequest = (() => {
202-
// Inner closure to actually test the URL.
203-
const urlTester: (url: string) => boolean = (() => {
204-
// On Android this function is documented to be skipped on first load:
205-
// therefore, simply never return true.
206-
if (Platform.OS === 'android') {
207-
return (url: string) => false;
208-
}
209-
210-
// Otherwise (for iOS), return a closure that evaluates to `true` _exactly
211-
// once_, and even then only if the URL looks like what we're expecting.
212-
let loaded_once = false;
213-
return (url: string) => {
214-
const parsedUrl = tryParseUrl(url);
215-
if (!loaded_once && parsedUrl && parsedUrl.toString() === baseUrl.toString()) {
216-
loaded_once = true;
217-
return true;
218-
}
219-
return false;
220-
};
221-
})();
222-
223-
// Outer closure to perform logging.
224-
return event => {
225-
const ok = urlTester(event.url);
226-
if (!ok) {
227-
logging.warn('webview: rejected navigation event', {
228-
navigation_event: { ...event },
229-
expected_url: baseUrl.toString(),
230-
});
231-
}
232-
return ok;
233-
};
234-
})();
235-
236-
// The `originWhitelist` and `onShouldStartLoadWithRequest` props are
237-
// meant to mitigate possible XSS bugs, by interrupting an attempted
238-
// exploit if it tries to navigate to a new URL by e.g. setting
239-
// `window.location`.
240-
//
241-
// Note that neither of them is a hard security barrier; they're checked
242-
// only against the URL of the document itself. They cannot be used to
243-
// validate the URL of other resources the WebView loads.
244-
//
245-
// Worse, the `originWhitelist` parameter is completely broken. See:
246-
// https://github.com/react-native-community/react-native-webview/pull/697
247-
return (
248-
<WebView
249-
source={{ baseUrl: (baseUrl.toString(): string), html }}
250-
originWhitelist={['file://']}
251-
onShouldStartLoadWithRequest={onShouldStartLoadWithRequest}
252-
decelerationRate="normal"
253-
style={{ backgroundColor: 'transparent' }}
254-
ref={this.webviewRef}
255-
onMessage={this.handleMessage}
256-
onError={this.handleError}
257-
/>
258-
);
198+
return renderSinglePageWebView(html, baseUrl, {
199+
decelerationRate: 'normal',
200+
style: { backgroundColor: 'transparent' },
201+
ref: this.webviewRef,
202+
onMessage: this.handleMessage,
203+
onError: this.handleError,
204+
});
259205
}
260206
}
261207

src/webview/SinglePageWebView.js

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// @flow strict-local
2+
import * as React from 'react';
3+
import { Platform } from 'react-native';
4+
import { WebView } from 'react-native-webview';
5+
6+
import * as logging from '../utils/logging';
7+
import { tryParseUrl } from '../utils/url';
8+
import type { ElementConfigFull } from '../reactUtils';
9+
10+
/**
11+
* Return a suitable onShouldStartLoadWithRequest for a single-page WebView.
12+
*
13+
* When passed as the onShouldStartLoadWithRequest prop to a WebView, the
14+
* returned callback will ensure that the webview never navigates away from
15+
* `baseUrl`.
16+
*
17+
* This is a hardening measure for our message-list WebView. We already
18+
* intercept clicks/touches and open links in a separate browser, but this
19+
* ensures that if something slips through that it still doesn't break our
20+
* security assumptions.
21+
*/
22+
// See upstream docs for this WebView prop:
23+
// https://github.com/react-native-webview/react-native-webview/blob/v11.22.2/docs/Reference.md#onshouldstartloadwithrequest
24+
function makeOnShouldStartLoadWithRequest(
25+
baseUrl: URL,
26+
): React.ElementConfig<typeof WebView>['onShouldStartLoadWithRequest'] {
27+
// Inner closure to actually test the URL.
28+
const urlTester: (url: string) => boolean = (() => {
29+
// On Android this function is documented to be skipped on first load:
30+
// therefore, simply never return true.
31+
if (Platform.OS === 'android') {
32+
return (url: string) => false;
33+
}
34+
35+
// Otherwise (for iOS), return a closure that evaluates to `true` _exactly
36+
// once_, and even then only if the URL looks like what we're expecting.
37+
let loaded_once = false;
38+
return (url: string) => {
39+
const parsedUrl = tryParseUrl(url);
40+
if (!loaded_once && parsedUrl && parsedUrl.toString() === baseUrl.toString()) {
41+
loaded_once = true;
42+
return true;
43+
}
44+
return false;
45+
};
46+
})();
47+
48+
// Outer closure to perform logging.
49+
return event => {
50+
const ok = urlTester(event.url);
51+
if (!ok) {
52+
logging.warn('webview: rejected navigation event', {
53+
navigation_event: { ...event },
54+
expected_url: baseUrl.toString(),
55+
});
56+
}
57+
return ok;
58+
};
59+
}
60+
61+
/**
62+
* Render a WebView that shows the given HTML at the given base URL, only.
63+
*
64+
* The WebView will show the page described by the HTML string `html`. Any
65+
* attempts to navigate to a new page will be rejected.
66+
*
67+
* Relative URL references to other resources (scripts, images, etc.) will
68+
* be resolved relative to `baseUrl`.
69+
*
70+
* Assumes `baseUrl` has the scheme `file:`. No actual file need exist at
71+
* `baseUrl` itself, because the page is taken from the string `html`.
72+
*/
73+
// TODO: This should ideally be a proper React component of its own. The
74+
// thing that may require care when doing that is our use of
75+
// `shouldComponentUpdate` in its caller, `MessageList`.
76+
export const renderSinglePageWebView = (
77+
html: string,
78+
baseUrl: URL,
79+
moreProps: $Rest<
80+
ElementConfigFull<typeof WebView>,
81+
{| source: mixed, originWhitelist: mixed, onShouldStartLoadWithRequest: mixed |},
82+
>,
83+
): React.Node => (
84+
// The `originWhitelist` and `onShouldStartLoadWithRequest` props are
85+
// meant to mitigate possible XSS bugs, by interrupting an attempted
86+
// exploit if it tries to navigate to a new URL by e.g. setting
87+
// `window.location`.
88+
//
89+
// Note that neither of them is a hard security barrier; they're checked
90+
// only against the URL of the document itself. They cannot be used to
91+
// validate the URL of other resources the WebView loads.
92+
//
93+
// Worse, the `originWhitelist` parameter is completely broken. See:
94+
// https://github.com/react-native-community/react-native-webview/pull/697
95+
<WebView
96+
source={{ baseUrl: (baseUrl.toString(): string), html: (html: string) }}
97+
originWhitelist={['file://']}
98+
onShouldStartLoadWithRequest={makeOnShouldStartLoadWithRequest(baseUrl)}
99+
{...moreProps}
100+
/>
101+
);

0 commit comments

Comments
 (0)