From 4d41025c8d0b819e3b23cfff507dd2e001e26b9e Mon Sep 17 00:00:00 2001 From: Daniel Sticker Date: Thu, 5 Nov 2026 02:28:23 +0100 Subject: [PATCH] fix(request): keep response headers and body out of errors A response that failed to parse as Thrift threw an error naming every response header and the body as hex. An object ID mismatch in uploadMediaByE2EE put every response header into its message and data. Those headers include x-line-next-access, the next access token, and callers log and print error messages. The parse error now names the status, method and body length. The object ID mismatch error names both object IDs. --- packages/linejs/base/obs/mod.ts | 15 +++--- packages/linejs/base/obs/upload_e2ee.test.ts | 33 +++++++++++++ packages/linejs/base/request/mod.test.ts | 51 ++++++++++++++++++++ packages/linejs/base/request/mod.ts | 6 ++- 4 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 packages/linejs/base/request/mod.test.ts diff --git a/packages/linejs/base/obs/mod.ts b/packages/linejs/base/obs/mod.ts index 7e005f5c..d0b4ca3f 100644 --- a/packages/linejs/base/obs/mod.ts +++ b/packages/linejs/base/obs/mod.ts @@ -356,20 +356,19 @@ export class LineObs { } else { previewEdata = edata; } - const { objId: objId2, headers } = await this.uploadObjectForService({ + const { objId: objId2 } = await this.uploadObjectForService({ data: previewEdata, oType: "file", obsPath: `${serviceName}/${obsNamespace}/${objId}__ud-preview`, params, }); if (objId !== objId2) { - throw new InternalError( - "ObsError", - "objId not match: " + JSON.stringify(Object.fromEntries(headers)), - { - headers: Object.fromEntries(headers), - }, - ); + // The response headers stay out: they can carry the next access + // token. Object IDs are no credential; fetching one needs the token. + throw new InternalError("ObsError", "objId not match", { + objId, + previewObjId: objId2, + }); } } diff --git a/packages/linejs/base/obs/upload_e2ee.test.ts b/packages/linejs/base/obs/upload_e2ee.test.ts index 80121522..82a4ea78 100644 --- a/packages/linejs/base/obs/upload_e2ee.test.ts +++ b/packages/linejs/base/obs/upload_e2ee.test.ts @@ -1,5 +1,7 @@ import { assert, test } from "vite-plus/test"; +import { assertRejects } from "../../test/assert.ts"; import { Buffer } from "node:buffer"; +import { InternalError } from "../core/mod.ts"; import { LineObs } from "./mod.ts"; /** Per-call record of (data.size, obsPath) so tests can assert which @@ -248,3 +250,34 @@ test("uploadMediaByE2EE — video duration preserves encrypted preview and messa assert.deepEqual(message.contentType, 2); assert.deepEqual(message.chunks, [new Uint8Array()]); }); + +// The preview upload answering a different object ID used to put every +// response header into the error, and those can carry the next access token. +test("uploadMediaByE2EE — an object ID mismatch names no response headers", async () => { + const nextAccess = "NEXT-ACCESS-MARKER"; + const { obs, fake } = makeObs(); + obs.uploadObjectForService = ((opts: { data: Blob; obsPath: string }) => { + fake.records.push({ size: opts.data.size, obsPath: opts.obsPath }); + return Promise.resolve({ + objId: `OBJ-${fake.records.length}`, + headers: new Headers({ "x-line-next-access": nextAccess }), + }); + }) as never; + + const error = await assertRejects( + () => + obs.uploadMediaByE2EE({ + data: new Blob([new Uint8Array(1_000)]), + oType: "image", + to: "u-recipient", + }), + InternalError, + ); + + assert.deepEqual(error.name, "ObsError"); + assert.notInclude(error.message, nextAccess); + assert.notInclude(JSON.stringify(error.data), nextAccess); + assert.deepEqual(error.data, { objId: "OBJ-1", previewObjId: "OBJ-2" }); + assert.deepEqual(fake.records.length, 2); + assert.deepEqual(fake.sendMessageCalls.length, 0); +}); diff --git a/packages/linejs/base/request/mod.test.ts b/packages/linejs/base/request/mod.test.ts new file mode 100644 index 00000000..36a7a8a9 --- /dev/null +++ b/packages/linejs/base/request/mod.test.ts @@ -0,0 +1,51 @@ +import { assert, test } from "vite-plus/test"; +import { assertRejects } from "../../test/assert.ts"; +import { BaseClient } from "../core/mod.ts"; + +const nextAccess = "NEXT-ACCESS-MARKER"; +const bodyText = "BODY-MARKER"; +const pathToken = "PATH-MARKER"; + +/** Every place a logger or error serializer reads an error from. */ +function errorText(error: Error): string { + return [ + error.message, + error.stack ?? "", + JSON.stringify({ ...error }), + String(error.cause ?? ""), + ].join("\n"); +} + +// A gateway error page is not Thrift, so the parse fails. That error used to +// print every response header, x-line-next-access included, and the body as +// hex, and callers copy the message into logs and tool output. A caller's +// path may carry a query string, so the error leaves the path out too. +test("request — an unparsable response names no headers, body or path", async () => { + const base = new BaseClient({ + device: "DESKTOPWIN", + fetch: () => + Promise.resolve( + new Response(bodyText, { + status: 503, + headers: { "x-line-next-access": nextAccess }, + }), + ), + }); + const body = new TextEncoder().encode(bodyText); + + const error = await assertRejects(() => + base.request.request([], "getProfile", 3, true, `/S3?token=${pathToken}`), + ); + + assert.strictEqual(Object.getPrototypeOf(error), Error.prototype); + const text = errorText(error); + assert.notInclude(text, nextAccess); + assert.notInclude(text, bodyText); + assert.notInclude(text, pathToken); + assert.notInclude(text, [...body].map((byte) => byte.toString(16)).join(" ")); + assert.deepEqual(error.cause, undefined); + assert.deepEqual( + error.message, + `Request internal failed: status=503 method=getProfile bodyLength=${body.length}`, + ); +}); diff --git a/packages/linejs/base/request/mod.ts b/packages/linejs/base/request/mod.ts index 6f84872a..ba087160 100644 --- a/packages/linejs/base/request/mod.ts +++ b/packages/linejs/base/request/mod.ts @@ -173,10 +173,12 @@ export class RequestClient { try { res = this.client.thrift.readThrift(parsedBody, protocol); } catch { + // Callers log and print this message, so it names no response + // headers (x-line-next-access carries the next access token), no + // body bytes and no path, which a caller may give a query string. throw new Error( `Request internal failed: status=${response.status} ` + - `headers=${JSON.stringify([...response.headers.entries()])} ` + - `body=<${[...parsedBody].map((e) => e.toString(16)).join(" ")}>`, + `method=${methodName} bodyLength=${parsedBody.length}`, ); } if (!res.data[0] && Object.keys(res.data).length) {