From 2d1cf1fafa2e3a7a53e971b97d4ffabbdf933d83 Mon Sep 17 00:00:00 2001 From: Simon Chan <1330321+yume-chan@users.noreply.github.com> Date: Fri, 8 Sep 2023 01:28:31 +0800 Subject: [PATCH] fix(webusb): try to discard lingering data fixes #597 --- libraries/adb-daemon-webusb/src/device.ts | 105 ++++++++++++++-------- libraries/adb/src/daemon/transport.ts | 20 +++-- libraries/android-bin/src/index.ts | 1 + 3 files changed, 84 insertions(+), 42 deletions(-) diff --git a/libraries/adb-daemon-webusb/src/device.ts b/libraries/adb-daemon-webusb/src/device.ts index c9519f49..88a99b95 100644 --- a/libraries/adb-daemon-webusb/src/device.ts +++ b/libraries/adb-daemon-webusb/src/device.ts @@ -99,6 +99,11 @@ class Uint8ArrayExactReadable implements ExactReadable { export class AdbDaemonWebUsbConnection implements ReadableWritablePair> { + #device: AdbDaemonWebUsbDevice; + get device() { + return this.#device; + } + #readable: ReadableStream; get readable() { return this.#readable; @@ -110,11 +115,13 @@ export class AdbDaemonWebUsbConnection } constructor( - device: USBDevice, + device: AdbDaemonWebUsbDevice, inEndpoint: USBEndpoint, outEndpoint: USBEndpoint, usbManager: USB, ) { + this.#device = device; + let closed = false; const duplex = new DuplexStreamFactory< @@ -124,7 +131,7 @@ export class AdbDaemonWebUsbConnection close: async () => { try { closed = true; - await device.close(); + await device.raw.close(); } catch { /* device may have already disconnected */ } @@ -139,7 +146,7 @@ export class AdbDaemonWebUsbConnection }); function handleUsbDisconnect(e: USBConnectionEvent) { - if (e.device === device) { + if (e.device === device.raw) { duplex.dispose().catch(unreachable); } } @@ -150,37 +157,63 @@ export class AdbDaemonWebUsbConnection new ReadableStream({ async pull(controller) { try { - // The `length` argument in `transferIn` must not be smaller than what the device sent, - // otherwise it will return `babble` status without any data. - // ADB daemon sends each packet in two parts, the 24-byte header and the payload. - const result = await device.transferIn( - inEndpoint.endpointNumber, - 24, - ); - - // TODO: webusb: handle `babble` by discarding the data and receive again - - // Per spec, the `result.data` always covers the whole `buffer`. - const buffer = new Uint8Array(result.data!.buffer); - const stream = new Uint8ArrayExactReadable(buffer); - - // Add `payload` field to its type, it's assigned below. - const packet = AdbPacketHeader.deserialize( - stream, - ) as AdbPacketHeader & { payload: Uint8Array }; - if (packet.payloadLength !== 0) { - const result = await device.transferIn( + while (true) { + // The `length` argument in `transferIn` must not be smaller than what the device sent, + // otherwise it will return `babble` status without any data. + // ADB daemon sends each packet in two parts, the 24-byte header and the payload. + const result = await device.raw.transferIn( inEndpoint.endpointNumber, - packet.payloadLength, + 24, ); - packet.payload = new Uint8Array( - result.data!.buffer, - ); - } else { - packet.payload = EMPTY_UINT8_ARRAY; - } - controller.enqueue(packet); + // Maximum payload size is 1MB, so reading 1MB data will always success, + // and always discards all lingering data. + // FIXME: Chrome on Windows doesn't support babble status. See the HACK below. + if (result.status === "babble") { + await device.raw.transferIn( + inEndpoint.endpointNumber, + 1024 * 1024, + ); + continue; + } + + // Per spec, the `result.data` always covers the whole `buffer`. + const buffer = new Uint8Array(result.data!.buffer); + const stream = new Uint8ArrayExactReadable(buffer); + + // Add `payload` field to its type, it's assigned below. + const packet = AdbPacketHeader.deserialize( + stream, + ) as AdbPacketHeader & { payload: Uint8Array }; + if (packet.payloadLength !== 0) { + // HACK: Chrome on Windows doesn't support babble status, + // so maybe we are not actually reading an ADB packet header. + // Currently the maximum payload size is 1MB, + // so if the payload length is larger than that, + // try to discard the data and receive again. + // https://crbug.com/1314358 + if (packet.payloadLength > 1024 * 1024) { + await device.raw.transferIn( + inEndpoint.endpointNumber, + 1024 * 1024, + ); + continue; + } + + const result = await device.raw.transferIn( + inEndpoint.endpointNumber, + packet.payloadLength, + ); + packet.payload = new Uint8Array( + result.data!.buffer, + ); + } else { + packet.payload = EMPTY_UINT8_ARRAY; + } + + controller.enqueue(packet); + return; + } } catch (e) { // On Windows, disconnecting the device will cause `NetworkError` to be thrown, // even before the `disconnect` event is fired. @@ -212,7 +245,7 @@ export class AdbDaemonWebUsbConnection new ConsumableWritableStream({ write: async (chunk) => { try { - await device.transferOut( + await device.raw.transferOut( outEndpoint.endpointNumber, chunk, ); @@ -225,7 +258,7 @@ export class AdbDaemonWebUsbConnection zeroMask && (chunk.byteLength & zeroMask) === 0 ) { - await device.transferOut( + await device.raw.transferOut( outEndpoint.endpointNumber, EMPTY_UINT8_ARRAY, ); @@ -281,9 +314,7 @@ export class AdbDaemonWebUsbDevice implements AdbDaemonDevice { * Claim the device and create a pair of `AdbPacket` streams to the ADB interface. * @returns The pair of `AdbPacket` streams. */ - async connect(): Promise< - ReadableWritablePair> - > { + async connect(): Promise { if (!this.#raw.opened) { await this.#raw.open(); } @@ -319,7 +350,7 @@ export class AdbDaemonWebUsbDevice implements AdbDaemonDevice { alternate.endpoints, ); return new AdbDaemonWebUsbConnection( - this.#raw, + this, inEndpoint, outEndpoint, this.#usbManager, diff --git a/libraries/adb/src/daemon/transport.ts b/libraries/adb/src/daemon/transport.ts index bf9869d3..4f9d8440 100644 --- a/libraries/adb/src/daemon/transport.ts +++ b/libraries/adb/src/daemon/transport.ts @@ -27,9 +27,14 @@ import { AdbCommand, calculateChecksum } from "./packet.js"; export const ADB_DAEMON_VERSION_OMIT_CHECKSUM = 0x01000001; +export type AdbDaemonConnection = ReadableWritablePair< + AdbPacketData, + Consumable +>; + interface AdbDaemonAuthenticationOptions { serial: string; - connection: ReadableWritablePair>; + connection: AdbDaemonConnection; credentialStore: AdbCredentialStore; authenticators?: AdbAuthenticator[]; /** @@ -40,7 +45,7 @@ interface AdbDaemonAuthenticationOptions { interface AdbDaemonSocketConnectorConstructionOptions { serial: string; - connection: ReadableWritablePair>; + connection: AdbDaemonConnection; version: number; maxPayloadSize: number; banner: string; @@ -94,9 +99,8 @@ export class AdbDaemonTransport implements AdbTransport { resolver.resolve(decodeUtf8(packet.payload)); break; case AdbCommand.Auth: { - const response = await authProcessor.process( - packet, - ); + const response = + await authProcessor.process(packet); await sendPacket(response); break; } @@ -193,6 +197,11 @@ export class AdbDaemonTransport implements AdbTransport { }); } + #connection: AdbDaemonConnection; + get connection() { + return this.#connection; + } + readonly #dispatcher: AdbPacketDispatcher; #serial: string; @@ -228,6 +237,7 @@ export class AdbDaemonTransport implements AdbTransport { preserveConnection, }: AdbDaemonSocketConnectorConstructionOptions) { this.#serial = serial; + this.#connection = connection; this.#banner = AdbBanner.parse(banner); let calculateChecksum: boolean; diff --git a/libraries/android-bin/src/index.ts b/libraries/android-bin/src/index.ts index 43917cc7..db729acc 100644 --- a/libraries/android-bin/src/index.ts +++ b/libraries/android-bin/src/index.ts @@ -9,3 +9,4 @@ export * from "./logcat.js"; export * from "./overlay-display.js"; export * from "./pm.js"; export * from "./settings.js"; +export * from "./string-format.js";