From c8f259ef4aefafba3fe002f0d09ccb1198037cc8 Mon Sep 17 00:00:00 2001 From: DrSmoothl <1787882683@qq.com> Date: Sat, 23 May 2026 13:06:09 +0800 Subject: [PATCH] test(highway): pin multi-image sequential semantics + richer 921 diag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User reported `highway upload error_code=921` when sending two images in one OneBot message — image 1 fast-uploaded (server already had the md5), image 2 (58 bytes) triggered the actual PUT and failed. Verified the claim from code inspection vs NapCatQQ + Lagrange.Core: - dev/NapCatQQ/.../highwayContext.ts:62-94 (checkAvailable + prepareUpload) - dev/NapCatQQ/.../highwayHttpUploader.ts:8-43 (HTTP block upload) - dev/NapCatQQ/.../highwayContext.ts:219-254 (uploadGroupImage) - dev/Lagrange.Core/.../HighwayContext.cs:98-180 (UploadSrcByStreamAsync) Findings: 1. The pipeline runs `runNtv2Upload` sequentially per image (await inside `buildSendElems`); there's no concurrency or shared state. 2. Each call gets its own OIDB request, its own lazy session fetch, its own buildHighwayExtend output. No aliasing. 3. The 921 comes from the highway PUT's response head (errorCode), not from anything client-emitted being malformed. Verdict: not reproducible from code inspection — almost certainly server-side (most likely Tencent rejecting the 58-byte image as a malformed payload; 58B is too small to even be a valid 1×1 PNG). Hardening: - Three regression tests pin the sequential two-image semantics so a future state-leak bug fails loudly: - extend bytes for image 2 must come from image 2's response, not image 1's, - each runNtv2Upload fetches its own non-null session, - each OIDB request carries its own fileInfo + a unique clientRandomId. - The `error_code=N` message now carries segRetCode, cmdId, chunk size, offset, and the first 8 bytes of the file md5 — without these the user couldn't tell apart "image rejected" vs "session expired" vs "rate-limited" on future reports. If the user reports the same 921 again, the new diagnostic message should show enough server-side context to triage server-vs-client. --- packages/bridge/src/highway/highway-client.ts | 15 +- .../core/tests/highway/multi-image.test.ts | 241 ++++++++++++++++++ 2 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 packages/core/tests/highway/multi-image.test.ts diff --git a/packages/bridge/src/highway/highway-client.ts b/packages/bridge/src/highway/highway-client.ts index 963a92dd..d5ad8226 100644 --- a/packages/bridge/src/highway/highway-client.ts +++ b/packages/bridge/src/highway/highway-client.ts @@ -239,7 +239,20 @@ export async function uploadHighwayHttp( const { head: respHead } = unpackHighwayFrame(responseBody); const resp = protobuf_decode(respHead); if (resp?.errorCode && resp.errorCode !== 0) { - throw new Error(`highway upload error_code=${resp.errorCode}`); + // Surface every diagnostic the highway response carries so + // user reports of `error_code=921` and friends include the + // server-side context (segHead.retCode, chunk size, file md5) + // — without these we can't tell apart a malformed-payload + // reject, a session-ticket mismatch, or a per-account rate- + // limit. + const segRetCode = resp.msgSegHead?.retCode ?? 0; + const fileMd5Hex = Buffer.from(fileMd5).toString('hex'); + throw new Error( + `highway upload error_code=${resp.errorCode}` + + ` (cmdId=${commandId} chunk=${chunkSize}/${bytes.length}` + + ` offset=${offset} segRetCode=${segRetCode}` + + ` fileMd5=${fileMd5Hex.slice(0, 16)}…)`, + ); } offset += chunkSize; console.log(`[Highway] uploaded ${offset}/${bytes.length} bytes`); diff --git a/packages/core/tests/highway/multi-image.test.ts b/packages/core/tests/highway/multi-image.test.ts new file mode 100644 index 00000000..05800821 --- /dev/null +++ b/packages/core/tests/highway/multi-image.test.ts @@ -0,0 +1,241 @@ +// Regression test for the "two images in one message" bug user report. +// +// User saw: 1st image fast-uploaded (server already had the bytes), 2nd +// image triggered an actual highway PUT and the highway server returned +// `error_code=921`. +// +// This test walks two `runNtv2Upload` calls back-to-back through the +// real pipeline + buildHighwayExtend code path with the highway TCP/HTTP +// transport stubbed, and verifies: +// +// - the two calls don't share state (each has its own session, its +// own OIDB request, its own extend bytes), +// - the second call's extend payload is well-formed (uKey + msgInfoBody +// + ipv4s come from the *second* call's response, not the first), +// - the highway PUT receives the second image's bytes + md5 + sha1 +// verbatim (no aliasing / truncation across calls). +// +// If this test ever starts failing, it's a real bug in the pipeline +// (state sharing, async race, or wrong-buffer aliasing). If it +// continues to pass, the user's 921 is almost certainly server-side +// (rate limit, malformed image rejection, expired session) rather than +// a client-emit issue. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { protobuf_decode, protobuf_encode } from '@snowluma/proton'; + +vi.mock('@snowluma/bridge/highway', async () => { + const real = await vi.importActual('@snowluma/bridge/highway'); + return { + ...real, + // Only the TCP/HTTP transport is mocked — we keep real + // `buildHighwayExtend` so we can inspect its output bytes. + fetchHighwaySession: vi.fn(async () => ({ + sigSession: new Uint8Array([0xAA]), + sessionKey: new Uint8Array([0xBB]), + host: 'highway.example', + port: 80, + })), + uploadHighwayHttp: vi.fn(async () => undefined), + }; +}); + +import * as highway from '@snowluma/bridge/highway'; +import { runNtv2Upload, type MediaSubFileUpload } from '@snowluma/bridge/highway/pipeline'; +import type { OidbBase } from '@snowluma/proto-defs/oidb'; +import type { + NTV2UploadRichMediaReq, + NTV2UploadRichMediaResp, + NTV2RichMediaHighwayExt, +} from '@snowluma/proto-defs/highway'; + +function encodeOidbResponse(body: any): Buffer { + return Buffer.from(protobuf_encode>({ + command: 0x11C4, subCommand: 100, errorCode: 0, + body: body as Record, errorMsg: '', reserved: 1, + })); +} + +function makeBridge(responses: Buffer[]) { + const sendRawPacket = vi.fn(); + for (const r of responses) { + sendRawPacket.mockResolvedValueOnce({ + success: true, gotResponse: true, errorCode: 0, errorMessage: '', + responseData: r, + }); + } + return { sendRawPacket, identity: { uin: '10001' } } as any; +} + +const baseParams = (bridge: any) => ({ + bridge, isGroup: true, targetIdOrUid: 894750076, + oidbCmd: 0x11C4, serviceCmd: 'OidbSvcTrpcTcp.0x11c4_100', + requestId: 1, businessType: 1, + compatQmsgSceneType: 2, + extBizInfo: { pic: { bizType: 0, textSummary: '[image]' } }, +}); + +describe('runNtv2Upload — two images in one message (sequential)', () => { + beforeEach(() => { + vi.mocked(highway.fetchHighwaySession).mockClear(); + vi.mocked(highway.uploadHighwayHttp).mockClear(); + }); + + it('does not leak the first image\'s upload metadata into the second\'s extend', async () => { + // Image 1 server response: NO uKey (fast-upload hit). The pipeline + // must skip the highway PUT entirely for this call. + // Image 2 server response: uKey + ipv4s + msgInfoBody set → real PUT. + const image1Bytes = Buffer.from(new Uint8Array(238963)); + const image1Md5 = new Uint8Array(16).fill(0xF4); + const image2Bytes = Buffer.from(new Uint8Array(58)); + const image2Md5 = new Uint8Array(16).fill(0xDD); + const image2Sha1 = new Uint8Array(20).fill(0x11); + + const bridge = makeBridge([ + // First OIDB upload response: fast-path hit (no uKey). + encodeOidbResponse({ + upload: { + msgInfo: { + msgInfoBody: [{ index: { fileUuid: 'uuid-1' } }], + extBizInfo: {}, + }, + }, + }), + // Second OIDB upload response: needs highway PUT. + encodeOidbResponse({ + upload: { + uKey: 'ukey-image-2', + ipv4s: [{ outIp: 0x04030201, outPort: 8080 }], + msgInfo: { + msgInfoBody: [{ index: { fileUuid: 'uuid-2', info: { fileHash: 'dd', fileSha1: '11' } } }], + extBizInfo: {}, + }, + }, + }), + ]); + + const uploads1: MediaSubFileUpload[] = [{ + source: 'top', cmdId: 1004, + bytes: image1Bytes, md5: image1Md5, sha1: new Uint8Array(20).fill(0xAB), + fastOnlyError: 'image fast-upload not available', + }]; + const uploads2: MediaSubFileUpload[] = [{ + source: 'top', cmdId: 1004, + bytes: image2Bytes, md5: image2Md5, sha1: image2Sha1, + fastOnlyError: 'image fast-upload not available', + }]; + + // Image 1 — fast path; must NOT touch the highway transport. + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ + fileInfo: { fileName: 'a.jpg', fileSize: 238963 }, + subFileType: 0, + }], + uploads: uploads1, + label: 'image', + }); + expect(highway.fetchHighwaySession).not.toHaveBeenCalled(); + expect(highway.uploadHighwayHttp).not.toHaveBeenCalled(); + + // Image 2 — must PUT through highway exactly once. + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ + fileInfo: { fileName: 'b.jpg', fileSize: 58 }, + subFileType: 0, + }], + uploads: uploads2, + label: 'image', + }); + expect(highway.fetchHighwaySession).toHaveBeenCalledOnce(); + expect(highway.uploadHighwayHttp).toHaveBeenCalledOnce(); + + // The PUT receives image-2's bytes, image-2's md5, and the + // image-2-derived extend (NOT image-1's data). + const [, , cmdId, putBytes, putMd5, putExtend] = vi.mocked(highway.uploadHighwayHttp).mock.calls[0]!; + expect(cmdId).toBe(1004); + expect(putBytes).toBe(image2Bytes); + expect(putMd5).toBe(image2Md5); + + // Decode the extend bytes; uKey + fileUuid must come from image-2's + // response, ipv4s must come from image-2's response. + const extend = protobuf_decode(putExtend); + expect(extend.uKey).toBe('ukey-image-2'); + expect(extend.fileUuid).toBe('uuid-2'); + expect(extend.msgInfoBody?.[0]?.index?.fileUuid).toBe('uuid-2'); + expect(extend.network?.ipv4s?.length).toBeGreaterThan(0); + }); + + it('image 2 fetches its own fresh session (no leftover from a prior call)', async () => { + // Both images hit the highway PUT path. The pipeline must call + // fetchHighwaySession once PER runNtv2Upload (not share session + // across calls), matching NapCat's per-context lifetime semantics + // would actually be a future optimisation, but for now correctness + // = "session is non-null when PUT runs". + const ok = encodeOidbResponse({ + upload: { + uKey: 'ukey', ipv4s: [{ outIp: 1, outPort: 80 }], + msgInfo: { msgInfoBody: [{ index: { fileUuid: 'u' } }], extBizInfo: {} }, + }, + }); + const bridge = makeBridge([ok, ok]); + + const uploads: MediaSubFileUpload[] = [{ + source: 'top', cmdId: 1004, + bytes: Buffer.from([1, 2, 3]), + md5: new Uint8Array(16), sha1: new Uint8Array(20), + fastOnlyError: 'image fast-upload not available', + }]; + + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ fileInfo: { fileName: 'a.jpg' }, subFileType: 0 }], + uploads, label: 'image', + }); + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ fileInfo: { fileName: 'b.jpg' }, subFileType: 0 }], + uploads, label: 'image', + }); + + expect(highway.fetchHighwaySession).toHaveBeenCalledTimes(2); + expect(highway.uploadHighwayHttp).toHaveBeenCalledTimes(2); + // The session object passed to the PUT must be non-null both times. + for (const call of vi.mocked(highway.uploadHighwayHttp).mock.calls) { + const session = call[1]; + expect(session).toBeDefined(); + expect((session as any).sigSession).toBeDefined(); + } + }); + + it('image 2\'s OIDB request body carries image-2\'s fileInfo (not image-1\'s)', async () => { + // Catches a hypothetical bug where someone mutates a shared body + // object across calls. Today the pipeline always builds a fresh + // body, but the regression is cheap to guard. + const ok = encodeOidbResponse({ + upload: { msgInfo: { msgInfoBody: [], extBizInfo: {} } }, + }); + const bridge = makeBridge([ok, ok]); + + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ fileInfo: { fileName: 'first.jpg', fileSize: 100 }, subFileType: 0 }], + uploads: [], label: 'image', + }); + await runNtv2Upload({ + ...baseParams(bridge), + uploadInfo: [{ fileInfo: { fileName: 'second.jpg', fileSize: 200 }, subFileType: 0 }], + uploads: [], label: 'image', + }); + + expect(bridge.sendRawPacket).toHaveBeenCalledTimes(2); + const body1 = protobuf_decode>(bridge.sendRawPacket.mock.calls[0]![1]); + const body2 = protobuf_decode>(bridge.sendRawPacket.mock.calls[1]![1]); + expect(body1.body?.upload?.uploadInfo?.[0]?.fileInfo?.fileName).toBe('first.jpg'); + expect(body2.body?.upload?.uploadInfo?.[0]?.fileInfo?.fileName).toBe('second.jpg'); + + // And the clientRandomId is different per call (sanity). + expect(body1.body?.upload?.clientRandomId).not.toBe(body2.body?.upload?.clientRandomId); + }); +});