fix(bin): properly escape arguments (#788)

libraries/adb/src/commands/subprocess/utils.ts

@@ -13,7 +13,8 @@ export function escapeArg(s: string) {
             break;
         }
         result += s.substring(base, found);
-        // a'b becomes a'\'b (the backslash is not a escape character)
+        // a'b becomes 'a'\'b', which is 'a' + \' + 'b'
+        // (quoted string 'a', escaped single quote, and quoted string 'b')
         result += String.raw`'\''`;
         base = found + 1;
     }
@@ -22,23 +23,32 @@ export function escapeArg(s: string) {
     return result;
 }
 
-export function splitCommand(command: string): string[] {
+/**
+ * Split the command.
+ *
+ * Quotes and escaped characters are supported, and will be returned as-is.
+ * @param input The input command
+ * @returns An array of string containing the arguments
+ */
+export function splitCommand(input: string): string[] {
     const result: string[] = [];
     let quote: string | undefined;
     let isEscaped = false;
     let start = 0;
 
-    for (let i = 0, len = command.length; i < len; i += 1) {
+    for (let i = 0, len = input.length; i < len; i += 1) {
         if (isEscaped) {
             isEscaped = false;
             continue;
         }
 
-        const char = command.charAt(i);
+        const char = input.charAt(i);
         switch (char) {
             case " ":
-                if (!quote && i !== start) {
+                if (!quote) {
-                    result.push(command.substring(start, i));
+                    if (i !== start) {
+                        result.push(input.substring(start, i));
+                    }
                     start = i + 1;
                 }
                 break;
@@ -56,8 +66,8 @@ export function splitCommand(command: string): string[] {
         }
     }
 
-    if (start < command.length) {
+    if (start < input.length) {
-        result.push(command.substring(start));
+        result.push(input.substring(start));
     }
 
     return result;

libraries/android-bin/src/am.ts

@@ -5,7 +5,7 @@ import { SplitStringStream, TextDecoderStream } from "@yume-chan/stream-extra";
 import { Cmd } from "./cmd/index.js";
 import type { IntentBuilder } from "./intent.js";
 import type { SingleUser } from "./utils.js";
-import { buildArguments } from "./utils.js";
+import { buildCommand } from "./utils.js";
 
 export interface ActivityManagerStartActivityOptions {
     displayId?: number;
@@ -43,21 +43,25 @@ export class ActivityManager extends AdbServiceBase {
     ): Promise<void> {
         // `am start` and `am start-activity` are the same,
         // but `am start-activity` was added in Android 8.
-        let args = buildArguments(
+        const command = buildCommand(
             [ActivityManager.ServiceName, "start", "-W"],
             options,
             START_ACTIVITY_OPTIONS_MAP,
         );
 
-        args = args.concat(options.intent.build().map(escapeArg));
+        for (const arg of options.intent.build()) {
+            command.push(arg);
+        }
 
         // `cmd activity` doesn't support `start` command on Android 7.
         let process: AdbNoneProtocolProcess;
         if (this.#apiLevel <= 25) {
-            args[0] = ActivityManager.CommandName;
+            command[0] = ActivityManager.CommandName;
-            process = await this.adb.subprocess.noneProtocol.spawn(args);
+            process = await this.adb.subprocess.noneProtocol.spawn(
+                command.map(escapeArg),
+            );
         } else {
-            process = await this.#cmd.spawn(args);
+            process = await this.#cmd.spawn(command);
         }
 
         const lines = process.output

libraries/android-bin/src/cmd/none.ts

@@ -3,6 +3,7 @@ import {
     AdbFeature,
     AdbNoneProtocolProcessImpl,
     adbNoneProtocolSpawner,
+    escapeArg,
 } from "@yume-chan/adb";
 
 import { Cmd } from "./service.js";
@@ -37,7 +38,7 @@ export function createNoneProtocol(
             spawn: adbNoneProtocolSpawner(async (command, signal) => {
                 checkCommand(command);
 
-                const newCommand = command.slice();
+                const newCommand = command.map(escapeArg);
                 newCommand.unshift("cmd");
                 return adb.subprocess.noneProtocol.spawn(newCommand, signal);
             }),
@@ -50,7 +51,7 @@ export function createNoneProtocol(
             spawn: adbNoneProtocolSpawner(async (command, signal) => {
                 checkCommand(command);
 
-                const newCommand = command.slice();
+                const newCommand = command.map(escapeArg);
                 newCommand[0] = resolveFallback(fallback, command[0]!);
                 return adb.subprocess.noneProtocol.spawn(newCommand, signal);
             }),

libraries/android-bin/src/cmd/shell.ts

@@ -3,6 +3,7 @@ import {
     AdbFeature,
     AdbShellProtocolProcessImpl,
     adbShellProtocolSpawner,
+    escapeArg,
 } from "@yume-chan/adb";
 
 import { Cmd } from "./service.js";
@@ -42,7 +43,7 @@ export function createShellProtocol(
             spawn: adbShellProtocolSpawner(async (command, signal) => {
                 checkCommand(command);
 
-                const newCommand = command.slice();
+                const newCommand = command.map(escapeArg);
                 newCommand.unshift("cmd");
                 return shellProtocolService.spawn(newCommand, signal);
             }),
@@ -55,7 +56,7 @@ export function createShellProtocol(
             spawn: adbShellProtocolSpawner(async (command, signal) => {
                 checkCommand(command);
 
-                const newCommand = command.slice();
+                const newCommand = command.map(escapeArg);
                 newCommand[0] = resolveFallback(fallback, command[0]!);
                 return shellProtocolService.spawn(newCommand, signal);
             }),

libraries/android-bin/src/cmd/utils.ts

@@ -1,5 +1,3 @@
-import { splitCommand } from "@yume-chan/adb";
-
 import type { Cmd } from "./service.js";
 
 export function resolveFallback(
@@ -31,13 +29,7 @@ export function serializeAbbService(
 ): string {
     checkCommand(command);
 
-    // `abb` mode doesn't use `sh -c` to execute to command,
-    // so it doesn't accept escaped arguments.
-    // `splitCommand` can be used to remove the escaping,
-    // each item in `command` must be a single argument.
-    const newCommand = command.map((arg) => splitCommand(arg)[0]!);
-
     // `abb` mode uses `\0` as the separator, allowing space in arguments.
     // The last `\0` is required for older versions of `adb`.
-    return `${prefix}:${newCommand.join("\0")}\0`;
+    return `${prefix}:${command.join("\0")}\0`;
 }

libraries/android-bin/src/pm.ts

@@ -16,7 +16,7 @@ import {
 import { Cmd } from "./cmd/index.js";
 import type { IntentBuilder } from "./intent.js";
 import type { Optional, SingleUserOrAll } from "./utils.js";
-import { buildArguments } from "./utils.js";
+import { buildCommand } from "./utils.js";
 
 export enum PackageManagerInstallLocation {
     Auto,
@@ -65,7 +65,7 @@ export const PackageManagerInstallOptions = {
     restrictPermissions: option<boolean>("--restrict-permissions", 29),
     doNotKill: option<boolean>("--dont-kill"),
     originatingUri: option<string>("--originating-uri"),
-    refererUri: option<string>("--referrer"),
+    referrerUri: option<string>("--referrer"),
     inheritFrom: option<string>("-p", 24),
     packageName: option<string>("--pkg", 28),
     abi: option<string>("--abi", 21),
@@ -187,7 +187,7 @@ const PACKAGE_MANAGER_RESOLVE_ACTIVITY_OPTIONS_MAP: Partial<
     user: "--user",
 };
 
-function buildInstallArguments(
+function buildInstallCommand(
     command: string,
     options: PackageManagerInstallOptions | undefined,
     apiLevel: number | undefined,
@@ -252,7 +252,7 @@ function buildInstallArguments(
                 args.push(option.name, value.toString());
                 break;
             case "string":
-                args.push(option.name, escapeArg(value));
+                args.push(option.name, value);
                 break;
             default:
                 throw new Error(
@@ -287,22 +287,26 @@ export class PackageManager extends AdbServiceBase {
         apks: readonly string[],
         options?: PackageManagerInstallOptions,
     ): Promise<void> {
-        const args = buildInstallArguments("install", options, this.#apiLevel);
+        const command = buildInstallCommand("install", options, this.#apiLevel);
-        args[0] = PackageManager.CommandName;
+
+        command[0] = PackageManager.CommandName;
+
         // WIP: old version of pm doesn't support multiple apks
-        args.push(...apks.map(escapeArg));
+        for (const apk of apks) {
+            command.push(apk);
+        }
 
         // Starting from Android 7, `pm` becomes a wrapper to `cmd package`.
         // The benefit of `cmd package` is it starts faster than the old `pm`,
         // because it connects to the already running `system` process,
         // instead of initializing all system components from scratch.
         //
-        // But launching `cmd package` directly causes it to not be able to
+        // But `cmd` executable can't read files in `/data/local/tmp`
-        // read files in `/data/local/tmp` (and many other places) due to SELinux policies,
+        // (and many other places) due to SELinux policies,
-        // so installing files must still use `pm`.
+        // so installing from files must still use `pm`.
         // (the starting executable file decides which SELinux policies to apply)
         const output = await this.adb.subprocess.noneProtocol
-            .spawn(args)
+            .spawn(command.map(escapeArg))
             .wait()
             .toString()
             .then((output) => output.trim());
@@ -352,9 +356,9 @@ export class PackageManager extends AdbServiceBase {
             return;
         }
 
-        const args = buildInstallArguments("install", options, this.#apiLevel);
+        const command = buildInstallCommand("install", options, this.#apiLevel);
-        args.push("-S", size.toString());
+        command.push("-S", size.toString());
-        const process = await this.#cmd.spawn(args);
+        const process = await this.#cmd.spawn(command);
 
         const output = process.output
             .pipeThrough(new TextDecoderStream())
@@ -429,17 +433,17 @@ export class PackageManager extends AdbServiceBase {
     async *listPackages(
         options?: Optional<PackageManagerListPackagesOptions>,
     ): AsyncGenerator<PackageManagerListPackagesResult, void, void> {
-        const args = buildArguments(
+        const command = buildCommand(
             ["package", "list", "packages"],
             options,
             PACKAGE_MANAGER_LIST_PACKAGES_OPTIONS_MAP,
         );
-        // `PACKAGE_MANAGER_LIST_PACKAGES_OPTIONS_MAP` doesn't have `filter`
+
         if (options?.filter) {
-            args.push(escapeArg(options.filter));
+            command.push(options.filter);
         }
 
-        const process = await this.#cmd.spawn(args);
+        const process = await this.#cmd.spawn(command);
 
         const output = process.output
             .pipeThrough(new TextDecoderStream())
@@ -473,19 +477,23 @@ export class PackageManager extends AdbServiceBase {
     ): Promise<string[]> {
         // `pm path` and `pm -p` are the same,
         // but `pm path` allows an optional `--user` option.
-        const args = [PackageManager.ServiceName, "path"];
+        const command = [PackageManager.ServiceName, "path"];
+
         if (options?.user !== undefined) {
-            args.push("--user", options.user.toString());
+            command.push("--user", options.user.toString());
         }
-        args.push(escapeArg(packageName));
+
+        command.push(packageName);
 
         // `cmd package` doesn't support `path` command on Android 7 and 8.
         let process: AdbNoneProtocolProcess;
         if (this.#apiLevel !== undefined && this.#apiLevel <= 27) {
-            args[0] = PackageManager.CommandName;
+            command[0] = PackageManager.CommandName;
-            process = await this.adb.subprocess.noneProtocol.spawn(args);
+            process = await this.adb.subprocess.noneProtocol.spawn(
+                command.map(escapeArg),
+            );
         } else {
-            process = await this.#cmd.spawn(args);
+            process = await this.#cmd.spawn(command);
         }
 
         const lines = process.output
@@ -510,18 +518,22 @@ export class PackageManager extends AdbServiceBase {
         packageName: string,
         options?: Optional<PackageManagerUninstallOptions>,
     ): Promise<void> {
-        let args = buildArguments(
+        const command = buildCommand(
             [PackageManager.ServiceName, "uninstall"],
             options,
             PACKAGE_MANAGER_UNINSTALL_OPTIONS_MAP,
         );
-        args.push(escapeArg(packageName));
+
+        command.push(packageName);
+
         if (options?.splitNames) {
-            args = args.concat(options.splitNames.map(escapeArg));
+            for (const splitName of options.splitNames) {
+                command.push(splitName);
+            }
         }
 
         const output = await this.#cmd
-            .spawn(args)
+            .spawn(command)
             .wait()
             .toString()
             .then((output) => output.trim());
@@ -533,16 +545,18 @@ export class PackageManager extends AdbServiceBase {
     async resolveActivity(
         options: PackageManagerResolveActivityOptions,
     ): Promise<string | undefined> {
-        let args = buildArguments(
+        const command = buildCommand(
             [PackageManager.ServiceName, "resolve-activity", "--components"],
             options,
             PACKAGE_MANAGER_RESOLVE_ACTIVITY_OPTIONS_MAP,
         );
 
-        args = args.concat(options.intent.build().map(escapeArg));
+        for (const arg of options.intent.build()) {
+            command.push(arg);
+        }
 
         const output = await this.#cmd
-            .spawn(args)
+            .spawn(command)
             .wait()
             .toString()
             .then((output) => output.trim());
@@ -567,14 +581,14 @@ export class PackageManager extends AdbServiceBase {
     async sessionCreate(
         options?: PackageManagerInstallOptions,
     ): Promise<number> {
-        const args = buildInstallArguments(
+        const command = buildInstallCommand(
             "install-create",
             options,
             this.#apiLevel,
         );
 
         const output = await this.#cmd
-            .spawn(args)
+            .spawn(command)
             .wait()
             .toString()
             .then((output) => output.trim());
@@ -605,16 +619,18 @@ export class PackageManager extends AdbServiceBase {
         splitName: string,
         path: string,
     ): Promise<void> {
-        const args: string[] = [
+        const command: string[] = [
             PackageManager.CommandName,
             "install-write",
             sessionId.toString(),
-            escapeArg(splitName),
+            splitName,
-            escapeArg(path),
+            path,
         ];
 
         // Similar to `install`, must use `adb.subprocess` so it can read `path`
-        const process = await this.adb.subprocess.noneProtocol.spawn(args);
+        const process = await this.adb.subprocess.noneProtocol.spawn(
+            command.map(escapeArg),
+        );
         await this.checkResult(process.output);
     }
 
@@ -624,17 +640,17 @@ export class PackageManager extends AdbServiceBase {
         size: number,
         stream: ReadableStream<MaybeConsumable<Uint8Array>>,
     ): Promise<void> {
-        const args: string[] = [
+        const command: string[] = [
             PackageManager.ServiceName,
             "install-write",
             "-S",
             size.toString(),
             sessionId.toString(),
-            escapeArg(splitName),
+            splitName,
             "-",
         ];
 
-        const process = await this.#cmd.spawn(args);
+        const process = await this.#cmd.spawn(command);
         await Promise.all([
             stream.pipeTo(process.stdin),
             this.checkResult(process.output),
@@ -647,7 +663,7 @@ export class PackageManager extends AdbServiceBase {
      * @returns A `Promise` that resolves when the session is committed
      */
     async sessionCommit(sessionId: number): Promise<void> {
-        const args: string[] = [
+        const command: string[] = [
             PackageManager.ServiceName,
             "install-commit",
             sessionId.toString(),
@@ -658,22 +674,24 @@ export class PackageManager extends AdbServiceBase {
         // causing this function to fail with an empty message.
         let process: AdbNoneProtocolProcess;
         if (this.#apiLevel !== undefined && this.#apiLevel <= 25) {
-            args[0] = PackageManager.CommandName;
+            command[0] = PackageManager.CommandName;
-            process = await this.adb.subprocess.noneProtocol.spawn(args);
+            process = await this.adb.subprocess.noneProtocol.spawn(
+                command.map(escapeArg),
+            );
         } else {
-            process = await this.#cmd.spawn(args);
+            process = await this.#cmd.spawn(command);
         }
 
         await this.checkResult(process.output);
     }
 
     async sessionAbandon(sessionId: number): Promise<void> {
-        const args: string[] = [
+        const command: string[] = [
             PackageManager.ServiceName,
             "install-abandon",
             sessionId.toString(),
         ];
-        const process = await this.#cmd.spawn(args);
+        const process = await this.#cmd.spawn(command);
         await this.checkResult(process.output);
     }
 }

libraries/android-bin/src/settings.ts

@@ -42,14 +42,17 @@ export class Settings extends AdbServiceBase {
         options: SettingsOptions | undefined,
         ...args: string[]
     ): Promise<string> {
-        let command = [Settings.ServiceName];
+        const command = [Settings.ServiceName];
 
         if (options?.user !== undefined) {
             command.push("--user", options.user.toString());
         }
 
         command.push(verb, namespace);
-        command = command.concat(args);
+
+        for (const arg of args) {
+            command.push(arg);
+        }
 
         return this.#cmd.spawn(command).wait().toString();
     }

libraries/android-bin/src/utils.ts

@@ -1,6 +1,4 @@
-import { escapeArg } from "@yume-chan/adb";
+export function buildCommand<T>(
-
-export function buildArguments<T>(
     commands: readonly string[],
     options: Partial<T> | undefined,
     map: Partial<Record<keyof T, string>>,
@@ -30,7 +28,7 @@ export function buildArguments<T>(
                     args.push(option, value.toString());
                     break;
                 case "string":
-                    args.push(option, escapeArg(value));
+                    args.push(option, value);
                     break;
                 default:
                     throw new Error(