From 15ec8e98dbffd00239c6651add48dd68436fec3b Mon Sep 17 00:00:00 2001 From: Yaash Singhh Date: Fri, 23 Oct 2026 03:28:31 +0530 Subject: [PATCH] fix(adb): improve shell output parsing accuracy (#5001) --- src/adb/AdbShellOutput.test.ts | 88 ++++++++++++++++++++++++++++++++++ src/adb/AdbShellOutput.ts | 37 +++++++++++--- 2 files changed, 119 insertions(+), 6 deletions(-) diff --git a/src/adb/AdbShellOutput.test.ts b/src/adb/AdbShellOutput.test.ts index 0d22c6b2c5..9af4debbaf 100644 --- a/src/adb/AdbShellOutput.test.ts +++ b/src/adb/AdbShellOutput.test.ts @@ -244,6 +244,36 @@ describe("parseDumpsysPackageOutput", () => { ).requestedPermissions, ).toBe(1); }); + + it("does not count permissions in the hidden system packages section", () => { + expect( + parseDumpsysPackageOutput( + [ + "Package [com.example] (abcdef):", + " pkg=Package{abcdef com.example}", + " requested permissions:", + " android.permission.INTERNET", + "Hidden system packages:", + " requested permissions:", + " com.android.hidden.permission", + ].join("\n"), + ).requestedPermissions, + ).toBe(1); + }); + + it("counts permission names containing hyphens and dollar signs", () => { + expect( + parseDumpsysPackageOutput( + [ + "Package [com.example] (abcdef):", + " pkg=Package{abcdef com.example}", + " requested permissions:", + " com.example.permission-Hyphen", + " com.example.permission$Dollar", + ].join("\n"), + ).requestedPermissions, + ).toBe(2); + }); }); describe("parseWmDensityOutput", () => { @@ -345,6 +375,64 @@ describe("parseAmStartOutput", () => { }); }); + it("refuses an unprefixed Java exception even when Starting is present", () => { + expect( + parseAmStartOutput( + `${starting}\njava.lang.SecurityException: Permission Denial\n`, + ), + ).toMatchObject({ + status: "refused", + reason: "java.lang.SecurityException: Permission Denial", + }); + }); + + it("refuses a message-free Java exception even when Starting is present", () => { + expect( + parseAmStartOutput(`${starting}\njava.lang.NullPointerException\n`), + ).toMatchObject({ + status: "refused", + reason: "java.lang.NullPointerException", + }); + }); + + it("recognizes a base Java exception type with a message", () => { + expect( + parseAmStartOutput(`${starting}\njava.lang.Exception: failure\n`), + ).toMatchObject({ + status: "refused", + reason: "java.lang.Exception: failure", + }); + }); + + it("prefers a specific Java exception over generic error headers", () => { + expect( + parseAmStartOutput( + [ + starting, + "Error type 3", + "Exception occurred while executing command", + "java.lang.SecurityException: Permission Denial", + ].join("\n"), + ), + ).toMatchObject({ + status: "refused", + reason: "java.lang.SecurityException: Permission Denial", + }); + + expect( + parseAmStartOutput( + [ + starting, + "Error: Generic launch failure", + "java.lang.SecurityException: Permission Denial", + ].join("\n"), + ), + ).toMatchObject({ + status: "refused", + reason: "java.lang.SecurityException: Permission Denial", + }); + }); + it("keeps Starting-only output compatible and refuses no-attempt output", () => { expect(parseAmStartOutput(starting)).toMatchObject({ status: "started", diff --git a/src/adb/AdbShellOutput.ts b/src/adb/AdbShellOutput.ts index 9ce558cebd..2ae876384f 100644 --- a/src/adb/AdbShellOutput.ts +++ b/src/adb/AdbShellOutput.ts @@ -316,6 +316,8 @@ export const parseDumpsysPackageOutput = ( let permissions = 0; let parsedPackageAvailable: boolean | null = null; for (const line of stdout.split(/\r?\n/u)) { + // Hidden system packages describe a separate section, not the requested package. + if (/^\s*Hidden system packages:\s*$/u.test(line)) break; if (line.trim().startsWith("requested permissions:")) { hasPermissionsSection = true; inPermissions = true; @@ -324,7 +326,7 @@ export const parseDumpsysPackageOutput = ( // Android 10 and 11 mark restricted entries as "NAME: restricted=true". if ( inPermissions && - /^\s{2,}[A-Za-z0-9._]+(?:: restricted=true)?$/u.test(line) + /^\s{2,}[A-Za-z0-9._$-]+(?:: restricted=true)?$/u.test(line) ) { permissions += 1; continue; @@ -465,15 +467,38 @@ export const parseAmStartOutput = ( ); const nonOkStatus = statuses.find((status) => status.toLowerCase() !== "ok"); const reportedStatus = nonOkStatus ?? statuses[0] ?? null; - const error = [normalizedStdout, stderr.replaceAll("\r\n", "\n")] + const errorLines = [normalizedStdout, stderr.replaceAll("\r\n", "\n")] .flatMap((stream) => stream.split("\n")) .map((line) => line.trim()) .map((line) => ({ line, - match: /^(?:Error|Exception)(?::|\b)(.*)$/iu.exec(line), - })) - .find(({ match }) => match !== null); - const errorReason = error?.match?.[1]?.trim() || error?.line || null; + prefixedMatch: /^(?:Error|Exception)(?::|\b)(.*)$/iu.exec(line), + throwable: + /^(?:(?:[A-Za-z_$][A-Za-z0-9_$]*\.)+(?:[A-Za-z_$][A-Za-z0-9_$]*(?:Exception|Error)|Exception|Error)|[A-Za-z_$][A-Za-z0-9_$]*(?:Exception|Error))(?::\s*.*)?$/u.test( + line, + ), + genericHeader: + /^(?:Error\s+type\s+\d+|Exception\s+occurred\s+while\s+executing\b.*)$/iu.test( + line, + ), + })); + + // Prefer the underlying Java throwable over the generic Activity Manager + // wrapper, regardless of which line or stream printed it first. + const error = + errorLines.find(({ throwable }) => throwable) ?? + errorLines.find( + ({ prefixedMatch, genericHeader }) => + prefixedMatch !== null && !genericHeader, + ) ?? + errorLines.find(({ prefixedMatch }) => prefixedMatch !== null); + + const errorReason = + error === undefined + ? null + : error.throwable + ? error.line + : error.prefixedMatch?.[1]?.trim() || error.line; const statusReason = nonOkStatus === "" ? "am start reported an empty status"