refactor(client): make parsing async and config an opaque string (#2324)

client/electron/go_helpers.ts

@@ -20,7 +20,6 @@
 
 import {pathToEmbeddedTun2socksBinary} from './app_paths';
 import {ChildProcessHelper} from './process';
-import {TransportConfigJson} from '../src/www/app/outline_server_repository/config';
 
 /**
  * Verifies the UDP connectivity of the server specified in `config`.
@@ -34,7 +33,7 @@ import {TransportConfigJson} from '../src/www/app/outline_server_repository/conf
  * @throws ProcessTerminatedExitCodeError if tun2socks failed to run.
  */
 export async function checkUDPConnectivity(
-  config: TransportConfigJson,
+  config: string,
   debugMode: boolean = false
 ): Promise<boolean> {
   const tun2socks = new ChildProcessHelper(pathToEmbeddedTun2socksBinary());
@@ -43,7 +42,7 @@ export async function checkUDPConnectivity(
   console.debug('[tun2socks] - checking connectivity ...');
   const output = await tun2socks.launch([
     '-transport',
-    JSON.stringify(config),
+    config,
     '-checkConnectivity',
   ]);
 

client/electron/go_vpn_tunnel.ts

@@ -21,7 +21,6 @@ import {checkUDPConnectivity} from './go_helpers';
 import {ChildProcessHelper, ProcessTerminatedSignalError} from './process';
 import {RoutingDaemon} from './routing_service';
 import {VpnTunnel} from './vpn_tunnel';
-import {TransportConfigJson} from '../src/www/app/outline_server_repository/config';
 import {TunnelStatus} from '../src/www/app/outline_server_repository/vpn';
 
 const isLinux = platform() === 'linux';
@@ -64,7 +63,7 @@ export class GoVpnTunnel implements VpnTunnel {
 
   constructor(
     private readonly routing: RoutingDaemon,
-    readonly transportConfig: TransportConfigJson
+    readonly transportConfig: string
   ) {
     this.tun2socks = new GoTun2socks();
 
@@ -248,10 +247,7 @@ class GoTun2socks {
    * Otherwise, an error containing a JSON-formatted message will be thrown.
    * @param isUdpEnabled Indicates whether the remote Outline server supports UDP.
    */
-  async start(
-    config: TransportConfigJson,
-    isUdpEnabled: boolean
-  ): Promise<void> {
+  async start(transportConfig: string, isUdpEnabled: boolean): Promise<void> {
     // ./tun2socks.exe \
     //   -tunName outline-tap0 -tunDNS 1.1.1.1,9.9.9.9 \
     //   -tunAddr 10.0.85.2 -tunGw 10.0.85.1 -tunMask 255.255.255.0 \
@@ -263,7 +259,7 @@ class GoTun2socks {
     args.push('-tunGw', TUN2SOCKS_VIRTUAL_ROUTER_IP);
     args.push('-tunMask', TUN2SOCKS_VIRTUAL_ROUTER_NETMASK);
     args.push('-tunDNS', DNS_RESOLVERS.join(','));
-    args.push('-transport', JSON.stringify(config));
+    args.push('-transport', transportConfig);
     args.push('-logLevel', this.process.isDebugModeEnabled ? 'debug' : 'info');
     if (!isUdpEnabled) {
       args.push('-dnsFallback');

client/electron/index.ts

@@ -19,6 +19,7 @@ import * as path from 'path';
 import * as process from 'process';
 import * as url from 'url';
 
+import * as net from '@outline/infrastructure/net';
 import * as Sentry from '@sentry/electron/main';
 import autoLaunch = require('auto-launch'); // tslint:disable-line
 import {
@@ -352,23 +353,26 @@ async function createVpnTunnel(
   // because startVpn will add a routing table entry that prefixed with this
   // host (e.g. "<host>/32"), therefore <host> must be an IP address.
   // TODO: make sure we resolve it in the native code
-  const host = tunnelConfig.firstHop.host;
+  const {host} = net.splitHostPort(tunnelConfig.firstHop);
   if (!host) {
     throw new errors.IllegalServerConfiguration('host is missing');
   }
   const hostIp = await lookupIp(host);
   const routing = new RoutingDaemon(hostIp || '', isAutoConnect);
   // Make sure the transport will use the IP we will allowlist.
-  const resolvedTransport =
-    config.setTransportConfigHost(tunnelConfig.transport, hostIp) ??
-    tunnelConfig.transport;
-  const tunnel = new GoVpnTunnel(routing, resolvedTransport);
+  // HACK: We do a simple string replacement in the config here. This may not always work with general configs
+  // but it works for simple configs.
+  // TODO: Remove the need to allowlisting the host IP.
+  tunnelConfig.transport = tunnelConfig.transport.replaceAll(host, hostIp);
+  const tunnel = new GoVpnTunnel(routing, tunnelConfig.transport);
   routing.onNetworkChange = tunnel.networkChanged.bind(tunnel);
   return tunnel;
 }
 
 // Invoked by both the start-proxying event handler and auto-connect.
 async function startVpn(request: StartRequestJson, isAutoConnect: boolean) {
+  console.debug('startVpn called with request ', JSON.stringify(request));
+
   if (IS_LINUX && !process.env.APPIMAGE) {
     onVpnStatusChanged((id, status) => {
       setUiTunnelStatus(status, id);

client/electron/vpn_service.ts

@@ -68,7 +68,7 @@ export async function establishVpn(request: StartRequestJson) {
     },
 
     // The actual transport config
-    transport: JSON.stringify(request.config.transport),
+    transport: request.config.transport,
   };
 
   await invokeMethod('EstablishVPN', JSON.stringify(config));

client/src/cordova/plugin/apple/src/OutlinePlugin.swift

@@ -151,16 +151,18 @@ class OutlinePlugin: CDVPlugin {
     guard let input = command.argument(at: 1) as? String else {
       return sendError("Missing method input", callbackId: command.callbackId)
     }
-    DDLogInfo("Invoking Method \(methodName) with input \(input)")
+    DDLogDebug("Invoking Method \(methodName) with input \(input)")
     Task {
       guard let result = OutlineInvokeMethod(methodName, input) else {
+        DDLogDebug("InvokeMethod \(methodName) got nil result")
         return self.sendError("unexpected invoke error", callbackId: command.callbackId)
       }
       if result.error != nil {
         let errorJson = marshalErrorJson(error: OutlineError.platformError(result.error!))
+        DDLogDebug("InvokeMethod \(methodName) failed with error \(errorJson)")
         return self.sendError(errorJson, callbackId: command.callbackId)
       }
-      DDLogInfo("InvokeMethod result: \(result.value)")
+      DDLogDebug("InvokeMethod result: \(result.value)")
       self.sendSuccess(result.value, callbackId: command.callbackId)
     }
   }

client/src/www/app/app.ts

@@ -18,7 +18,6 @@ import {OperationTimedOut} from '@outline/infrastructure/timeout_promise';
 import {Clipboard} from './clipboard';
 import {EnvironmentVariables} from './environment';
 import {localizeErrorCode} from './error_localizer';
-import {OutlineServerRepository} from './outline_server_repository';
 import * as config from './outline_server_repository/config';
 import {Settings, SettingsKey} from './settings';
 import {Updater} from './updater';
@@ -30,7 +29,7 @@ import {
   PlatformError,
   ROUTING_SERVICE_NOT_RUNNING,
 } from '../model/platform_error';
-import {Server} from '../model/server';
+import {Server, ServerRepository} from '../model/server';
 import {OutlineErrorReporter} from '../shared/error_reporter';
 import {ServerConnectionState, ServerListItem} from '../views/servers_view';
 import {SERVER_CONNECTION_INDICATOR_DURATION_MS} from '../views/servers_view/server_connection_indicator';
@@ -94,7 +93,7 @@ export class App {
 
   constructor(
     private eventQueue: events.EventQueue,
-    private serverRepo: OutlineServerRepository,
+    private serverRepo: ServerRepository,
     private rootEl: polymer.Base,
     private debugMode: boolean,
     urlInterceptor: UrlInterceptor | undefined,
@@ -227,9 +226,11 @@ export class App {
 
     this.eventQueue.startPublishing();
 
-    this.rootEl.$.addServerView.isValidAccessKey = (accessKey: string) => {
+    this.rootEl.$.addServerView.validateAccessKey = async (
+      accessKey: string
+    ): Promise<boolean> => {
       try {
-        config.parseAccessKey(accessKey);
+        await config.parseAccessKey(accessKey);
         return true;
       } catch {
         return false;
@@ -363,7 +364,7 @@ export class App {
   private async pullClipboardText() {
     try {
       const text = await this.clipboard.getContents();
-      this.handleClipboardText(text);
+      await this.handleClipboardText(text);
     } catch (e) {
       console.warn('cannot read clipboard, system may lack clipboard support');
     }
@@ -416,13 +417,13 @@ export class App {
     this.rootEl.changePage(event.detail.page);
   }
 
-  private handleClipboardText(text: string) {
+  private async handleClipboardText(text: string) {
     // Shorten, sanitise.
     // Note that we always check the text, even if the contents are same as last time, because we
     // keep an in-memory cache of user-ignored access keys.
     text = text.substring(0, 1000).trim();
     try {
-      this.confirmAddServer(text, true);
+      await this.confirmAddServer(text, true);
     } catch (err) {
       // Don't alert the user; high false positive rate.
     }
@@ -444,28 +445,29 @@ export class App {
   }
 
   private requestAddServer(event: CustomEvent) {
-    try {
-      this.serverRepo.add(event.detail.accessKey);
-    } catch (err) {
-      this.changeToDefaultPage();
-      this.showLocalizedError(err);
-    } finally {
-      this.rootEl.$.addServerView.open = false;
-    }
+    this.serverRepo
+      .add(event.detail.accessKey)
+      .catch(err => {
+        this.changeToDefaultPage();
+        this.showLocalizedError(err);
+      })
+      .finally(() => {
+        this.rootEl.$.addServerView.open = false;
+      });
   }
 
-  private requestAddServerConfirmation(event: CustomEvent) {
+  private async requestAddServerConfirmation(event: CustomEvent) {
     const accessKey = event.detail.accessKey;
     console.debug('Got add server confirmation request from UI');
     try {
-      this.confirmAddServer(accessKey);
+      await this.confirmAddServer(accessKey);
     } catch (err) {
       console.error('Failed to confirm add sever.', err);
       this.showLocalizedError(err);
     }
   }
 
-  private confirmAddServer(accessKey: string, fromClipboard = false) {
+  private async confirmAddServer(accessKey: string, fromClipboard = false) {
     const addServerView = this.rootEl.$.addServerView;
     accessKey = unwrapInvite(accessKey);
     if (fromClipboard && !addServerView.open) {
@@ -478,7 +480,7 @@ export class App {
       }
     }
     try {
-      config.validateAccessKey(accessKey);
+      await config.parseAccessKey(accessKey);
       addServerView.accessKey = accessKey;
       addServerView.open = true;
     } catch (e) {
@@ -821,7 +823,7 @@ export class App {
   }
 
   private registerUrlInterceptionListener(urlInterceptor: UrlInterceptor) {
-    urlInterceptor.registerListener(url => {
+    urlInterceptor.registerListener(async url => {
       if (!isOutlineAccessKey(unwrapInvite(url))) {
         // This check is necessary to ignore empty and malformed install-referrer URLs in Android
         // while allowing ss://, ssconf:// and invite URLs.
@@ -830,7 +832,7 @@ export class App {
       }
 
       try {
-        this.confirmAddServer(url);
+        await this.confirmAddServer(url);
       } catch (err) {
         this.showLocalizedErrorInDefaultPage(err);
       }

client/src/www/app/main.cordova.ts

@@ -76,7 +76,12 @@ class CordovaErrorReporter extends SentryErrorReporter {
 
 class CordovaMethodChannel implements MethodChannel {
   invokeMethod(methodName: string, params: string): Promise<string> {
-    return pluginExecWithErrorCode('invokeMethod', methodName, params);
+    try {
+      return pluginExecWithErrorCode('invokeMethod', methodName, params);
+    } catch (e) {
+      console.debug('invokeMethod failed', methodName, e);
+      throw e;
+    }
   }
 }
 

client/src/www/app/main.ts

@@ -19,7 +19,7 @@ import {makeConfig, SIP002_URI} from 'ShadowsocksConfig';
 
 import {App} from './app';
 import {onceEnvVars} from './environment';
-import {OutlineServerRepository} from './outline_server_repository';
+import {newOutlineServerRepository} from './outline_server_repository';
 import {
   FAKE_BROKEN_HOSTNAME,
   FAKE_UNREACHABLE_HOSTNAME,
@@ -28,6 +28,7 @@ import {
 import {OutlinePlatform} from './platform';
 import {Settings} from './settings';
 import {EventQueue} from '../model/events';
+import {ServerRepository} from '../model/server.js';
 
 // Used to determine whether to use Polymer functionality on app initialization failure.
 let webComponentsAreReady = false;
@@ -52,11 +53,14 @@ function getRootEl() {
   return document.querySelector('app-root') as {} as polymer.Base;
 }
 
-function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {
+async function createServerRepo(
+  platform: OutlinePlatform,
+  eventQueue: EventQueue
+): Promise<ServerRepository> {
   const localize = getLocalizationFunction();
   const vpnApi = platform.getVpnApi();
   if (vpnApi) {
-    return new OutlineServerRepository(
+    return await newOutlineServerRepository(
       vpnApi,
       eventQueue,
       window.localStorage,
@@ -65,7 +69,7 @@ function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {
   }
 
   console.debug('Platform not supported, using fake servers.');
-  const repo = new OutlineServerRepository(
+  const repo = await newOutlineServerRepository(
     new FakeVpnApi(),
     eventQueue,
     window.localStorage,
@@ -109,14 +113,14 @@ function createServerRepo(platform: OutlinePlatform, eventQueue: EventQueue) {
 
 export function main(platform: OutlinePlatform) {
   return Promise.all([onceEnvVars, oncePolymerIsReady]).then(
-    ([environmentVars]) => {
+    async ([environmentVars]) => {
       console.debug('running main() function');
 
       const queryParams = new URL(document.URL).searchParams;
       const debugMode = queryParams.get('debug') === 'true';
 
       const eventQueue = new EventQueue();
-      const serverRepo = createServerRepo(platform, eventQueue);
+      const serverRepo = await createServerRepo(platform, eventQueue);
       const settings = new Settings();
       new App(
         eventQueue,