msglist [nfc]: Separate out "single-page webview" code from "message list" code

src/reactUtils.js

@@ -1,6 +1,20 @@
 /* @flow strict-local */
 import invariant from 'invariant';
-import { useRef, useEffect, useState } from 'react';
+import * as React from 'react';
+
+/**
+ * Like React.ElementConfig, but includes the pseudoprops `ref` and `key`.
+ *
+ * That is, this contains exactly the set of JSX attributes one can pass
+ * when creating an element of this component-type.
+ *
+ * Assumes the underlying props type is an exact object type.
+ */
+export type ElementConfigFull<+C> = {|
+  ...$Exact<React.ElementConfig<C>>,
+  +ref?: React.Ref<C>,
+  +key?: React.Key,
+|};
 
 /**
  * A Hook for the value of a prop, state, etc., from the previous render.
@@ -25,8 +39,8 @@ import { useRef, useEffect, useState } from 'react';
 // (because effectively the `?` would handle it instead), and so `U` would
 // be the empty type and `T | U` would be just `T`.
 export function usePrevious<T, U>(value: T, initValue: U): T | U {
-  const ref = useRef<T | U>(initValue);
-  useEffect(() => {
+  const ref = React.useRef<T | U>(initValue);
+  React.useEffect(() => {
     ref.current = value;
   });
   return ref.current;
@@ -48,7 +62,7 @@ export function useDebugAssertConstant<T>(value: T) {
 
   // Conditional, but on a per-process constant.
   // eslint-disable-next-line react-hooks/rules-of-hooks
-  const origValue = useRef(value);
+  const origValue = React.useRef(value);
   invariant(value === origValue.current, '');
 }
 
@@ -71,9 +85,9 @@ export function useDebugAssertConstant<T>(value: T) {
 export function useHasNotChangedForMs(value: mixed, duration: number): boolean {
   useDebugAssertConstant(duration);
 
-  const [result, setResult] = useState(false);
+  const [result, setResult] = React.useState(false);
 
-  useEffect(() => {
+  React.useEffect(() => {
     setResult(false);
     const id = setTimeout(() => setResult(true), duration);
     return () => clearTimeout(id);
@@ -134,4 +148,4 @@ export const useHasStayedTrueForMs = (value: boolean, duration: number): boolean
 // docs could be clearer about that:
 //   https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects
 export const useConditionalEffect = (cb: () => void | (() => void), value: boolean): void =>
-  useEffect(() => (value ? cb() : undefined), [value, cb]);
+  React.useEffect(() => (value ? cb() : undefined), [value, cb]);

src/webview/MessageList.js

@@ -1,5 +1,5 @@
 /* @flow strict-local */
-import React, { Component, type ComponentType } from 'react';
+import * as React from 'react';
 import { Platform, NativeModules } from 'react-native';
 import { WebView } from 'react-native-webview';
 
@@ -33,11 +33,10 @@ import messageListElementHtml from './html/messageListElementHtml';
 import generateInboundEvents from './generateInboundEvents';
 import { handleWebViewOutboundEvent } from './handleOutboundEvents';
 import { base64Utf8Encode } from '../utils/encoding';
-import * as logging from '../utils/logging';
-import { tryParseUrl } from '../utils/url';
 import { caseNarrow, isConversationNarrow } from '../utils/narrow';
 import { type BackgroundData, getBackgroundData } from './backgroundData';
 import { ensureUnreachable } from '../generics';
+import { renderSinglePageWebView } from './SinglePageWebView';
 
 type OuterProps = $ReadOnly<{|
   narrow: Narrow,
@@ -76,20 +75,27 @@ export type Props = $ReadOnly<{|
 /**
  * The URL of the platform-specific assets folder.
  *
- * - On iOS: We can't easily hardcode this because it includes UUIDs.
- *   So we bring it over the React Native bridge in ZLPConstants.m.
- *
- * - On Android: Different apps' WebViews see different (virtual) root
- *   directories as `file:///`, and in particular the WebView provides
- *   the APK's `assets/` directory as `file:///android_asset/`. [1]
- *   We can easily hardcode that, so we do.
- *
- * [1] Oddly, this essential feature doesn't seem to be documented!  It's
- *     widely described in how-tos across the web and StackOverflow answers.
- *     It's assumed in some related docs which mention it in passing, and
- *     treated matter-of-factly in some Chromium bug threads.  Details at:
- *     https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/android.20filesystem/near/796440
+ * This will be a `file:` URL.
  */
+// It could be perfectly reasonable for this to be an `http:` or `https:`
+// URL instead, at least in development.  We'd then just need to adjust
+// the `originWhitelist` we pass to `WebView`.
+//
+// - On iOS: We can't easily hardcode this because it includes UUIDs.
+//   So we bring it over the React Native bridge in ZLPConstants.m.
+//   It's a file URL because the app bundle's `resourceURL` is:
+//     https://developer.apple.com/documentation/foundation/bundle/1414821-resourceurl
+//
+// - On Android: Different apps' WebViews see different (virtual) root
+//   directories as `file:///`, and in particular the WebView provides
+//   the APK's `assets/` directory as `file:///android_asset/`. [1]
+//   We can easily hardcode that, so we do.
+//
+// [1] Oddly, this essential feature doesn't seem to be documented!  It's
+//     widely described in how-tos across the web and StackOverflow answers.
+//     It's assumed in some related docs which mention it in passing, and
+//     treated matter-of-factly in some Chromium bug threads.  Details at:
+//     https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/android.20filesystem/near/796440
 const assetsUrl =
   Platform.OS === 'ios'
     ? new URL(NativeModules.ZLPConstants.resourceURL)
@@ -102,7 +108,21 @@ const assetsUrl =
  */
 const webviewAssetsUrl = new URL('webview/', assetsUrl);
 
-class MessageListInner extends Component<Props> {
+/**
+ * Effective URL of the MessageList webview.
+ *
+ * It points to `index.html` in the webview-assets folder, which
+ * doesn't exist.
+ *
+ * It doesn't need to exist because we provide all HTML at
+ * creation (or refresh) time. This serves only as a placeholder,
+ * so that relative URLs (e.g., to `base.css`, which does exist)
+ * and cross-domain security restrictions have somewhere to
+ * believe that this document originates from.
+ */
+const baseUrl = new URL('index.html', webviewAssetsUrl);
+
+class MessageListInner extends React.Component<Props> {
   webviewRef = React.createRef<React$ElementRef<typeof WebView>>();
   sendInboundEventsIsReady: boolean;
   unsentInboundEvents: WebViewInboundEvent[] = [];
@@ -154,13 +174,7 @@ class MessageListInner extends Component<Props> {
       _,
     } = this.props;
     const contentHtml = messageListElementsForShownMessages
-      .map(element =>
-        messageListElementHtml({
-          backgroundData,
-          element,
-          _,
-        }),
-      )
+      .map(element => messageListElementHtml({ backgroundData, element, _ }))
       .join('');
     const { auth, theme } = backgroundData;
     const html: string = getHtml(
@@ -175,80 +189,13 @@ class MessageListInner extends Component<Props> {
       backgroundData.serverEmojiData,
     );
 
-    /**
-     * Effective URL of the MessageList webview.
-     *
-     * It points to `index.html` in the webview-assets folder, which
-     * doesn't exist.
-     *
-     * It doesn't need to exist because we provide all HTML at
-     * creation (or refresh) time. This serves only as a placeholder,
-     * so that relative URLs (e.g., to `base.css`, which does exist)
-     * and cross-domain security restrictions have somewhere to
-     * believe that this document originates from.
-     */
-    const baseUrl = new URL('index.html', webviewAssetsUrl);
-
-    // Paranoia^WSecurity: only load `baseUrl`, and only load it once. Any other
-    // requests should be handed off to the OS, not loaded inside the WebView.
-    const onShouldStartLoadWithRequest = (() => {
-      // Inner closure to actually test the URL.
-      const urlTester: (url: string) => boolean = (() => {
-        // On Android this function is documented to be skipped on first load:
-        // therefore, simply never return true.
-        if (Platform.OS === 'android') {
-          return (url: string) => false;
-        }
-
-        // Otherwise (for iOS), return a closure that evaluates to `true` _exactly
-        // once_, and even then only if the URL looks like what we're expecting.
-        let loaded_once = false;
-        return (url: string) => {
-          const parsedUrl = tryParseUrl(url);
-          if (!loaded_once && parsedUrl && parsedUrl.toString() === baseUrl.toString()) {
-            loaded_once = true;
-            return true;
-          }
-          return false;
-        };
-      })();
-
-      // Outer closure to perform logging.
-      return event => {
-        const ok = urlTester(event.url);
-        if (!ok) {
-          logging.warn('webview: rejected navigation event', {
-            navigation_event: { ...event },
-            expected_url: baseUrl.toString(),
-          });
-        }
-        return ok;
-      };
-    })();
-
-    // The `originWhitelist` and `onShouldStartLoadWithRequest` props are
-    // meant to mitigate possible XSS bugs, by interrupting an attempted
-    // exploit if it tries to navigate to a new URL by e.g. setting
-    // `window.location`.
-    //
-    // Note that neither of them is a hard security barrier; they're checked
-    // only against the URL of the document itself.  They cannot be used to
-    // validate the URL of other resources the WebView loads.
-    //
-    // Worse, the `originWhitelist` parameter is completely broken. See:
-    // https://github.com/react-native-community/react-native-webview/pull/697
-    return (
-      <WebView
-        source={{ baseUrl: (baseUrl.toString(): string), html }}
-        originWhitelist={['file://']}
-        onShouldStartLoadWithRequest={onShouldStartLoadWithRequest}
-        decelerationRate="normal"
-        style={{ backgroundColor: 'transparent' }}
-        ref={this.webviewRef}
-        onMessage={this.handleMessage}
-        onError={this.handleError}
-      />
-    );
+    return renderSinglePageWebView(html, baseUrl, {
+      decelerationRate: 'normal',
+      style: { backgroundColor: 'transparent' },
+      ref: this.webviewRef,
+      onMessage: this.handleMessage,
+      onError: this.handleError,
+    });
   }
 }
 
@@ -283,7 +230,7 @@ const marksMessagesAsRead = (narrow: Narrow): boolean =>
     mentioned: () => false,
   });
 
-const MessageList: ComponentType<OuterProps> = connect<SelectorProps, _, _>(
+const MessageList: React.ComponentType<OuterProps> = connect<SelectorProps, _, _>(
   (state, props: OuterProps) => {
     // If this were a function component with Hooks, these would be
     // useGlobalSelector calls and would coexist perfectly smoothly with

src/webview/SinglePageWebView.js

@@ -0,0 +1,99 @@
+// @flow strict-local
+import * as React from 'react';
+import { Platform } from 'react-native';
+import { WebView } from 'react-native-webview';
+
+import * as logging from '../utils/logging';
+import { tryParseUrl } from '../utils/url';
+import type { ElementConfigFull } from '../reactUtils';
+
+/**
+ * Return a suitable onShouldStartLoadWithRequest for a single-page WebView.
+ *
+ * When passed as the onShouldStartLoadWithRequest prop to a WebView, the
+ * returned callback will ensure that the webview never navigates away from
+ * `baseUrl`.
+ *
+ * This is a hardening measure for our message-list WebView.  We already
+ * intercept clicks/touches and open links in a separate browser, but this
+ * ensures that if something slips through that it still doesn't break our
+ * security assumptions.
+ */
+// See upstream docs for this WebView prop:
+//   https://github.com/react-native-webview/react-native-webview/blob/v11.22.2/docs/Reference.md#onshouldstartloadwithrequest
+function makeOnShouldStartLoadWithRequest(
+  baseUrl: URL,
+): React.ElementConfig<typeof WebView>['onShouldStartLoadWithRequest'] {
+  let loaded_once = false;
+
+  return event => {
+    // eslint-disable-next-line no-use-before-define
+    const ok = urlIsOk(event.url);
+    if (!ok) {
+      logging.warn('webview: rejected navigation event', {
+        navigation_event: { ...event },
+        expected_url: baseUrl.toString(),
+      });
+    }
+    return ok;
+  };
+
+  function urlIsOk(url: string): boolean {
+    // On Android the onShouldStartLoadWithRequest prop is documented to be
+    // skipped on first load; therefore, simply never return true.
+    if (Platform.OS === 'android') {
+      return false;
+    }
+
+    // Otherwise (for iOS), return `true` only if the URL looks like what
+    // we're expecting, and only the first such time.
+    const parsedUrl = tryParseUrl(url);
+    if (!loaded_once && parsedUrl && parsedUrl.toString() === baseUrl.toString()) {
+      loaded_once = true;
+      return true;
+    }
+    return false;
+  }
+}
+
+/**
+ * Render a WebView that shows the given HTML at the given base URL, only.
+ *
+ * The WebView will show the page described by the HTML string `html`.  Any
+ * attempts to navigate to a new page will be rejected.
+ *
+ * Relative URL references to other resources (scripts, images, etc.) will
+ * be resolved relative to `baseUrl`.
+ *
+ * Assumes `baseUrl` has the scheme `file:`.  No actual file need exist at
+ * `baseUrl` itself, because the page is taken from the string `html`.
+ */
+// TODO: This should ideally be a proper React component of its own.  The
+//       thing that may require care when doing that is our use of
+//       `shouldComponentUpdate` in its caller, `MessageList`.
+export const renderSinglePageWebView = (
+  html: string,
+  baseUrl: URL,
+  moreProps: $Rest<
+    ElementConfigFull<typeof WebView>,
+    {| source: mixed, originWhitelist: mixed, onShouldStartLoadWithRequest: mixed |},
+  >,
+): React.Node => (
+  // The `originWhitelist` and `onShouldStartLoadWithRequest` props are
+  // meant to mitigate possible XSS bugs, by interrupting an attempted
+  // exploit if it tries to navigate to a new URL by e.g. setting
+  // `window.location`.
+  //
+  // Note that neither of them is a hard security barrier; they're checked
+  // only against the URL of the document itself.  They cannot be used to
+  // validate the URL of other resources the WebView loads.
+  //
+  // Worse, the `originWhitelist` parameter is completely broken. See:
+  // https://github.com/react-native-community/react-native-webview/pull/697
+  <WebView
+    source={{ baseUrl: (baseUrl.toString(): string), html: (html: string) }}
+    originWhitelist={['file://']}
+    onShouldStartLoadWithRequest={makeOnShouldStartLoadWithRequest(baseUrl)}
+    {...moreProps}
+  />
+);