From caf68aaea7e3b623a1b85d49cd9fca4ad9f46b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 7 Jun 2026 11:23:34 +0200 Subject: [PATCH 1/2] fix(import): stop the multipart writer from dropping a part body on chunk boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-rolled multipart parser consumed the opening boundary marker and then inspected the next two bytes inline to classify the suffix ("\r\n" for another part, "--" for the closing boundary). When a stream chunk ended exactly on that marker, fewer than two bytes were left in the buffer, so the branch returned while still in the "preamble" state. The next read re-entered preamble, searched for the marker again — which was already consumed — and treated the part headers and the entire file body as preamble to discard, matching only at the closing boundary. The captured file landed on disk truncated or empty. A tiny single-chunk upload never tripped it; a multi-megabyte binary arrives in hundreds of chunks, so a chunk boundary routinely fell in the dead window and the saved file came up short. The Apple Health export ingest then failed its ZIP read with "Could not locate ZIP End-Of-Central-Directory record". Add a dedicated "after-boundary" parser state that buffers until the two-byte suffix is present instead of falling back to a marker search, route the post-body separator through the same state, guard the epilogue state against the body scanner, and tolerate RFC 2046 transport padding after the marker. The preamble retain window now keeps boundaryMarker.length - 1 bytes so a marker split across chunks can still complete. Streaming and the size cap are unchanged — no part of the upload is buffered whole. Cover the regression with byte-exact round-trip tests over adversarial chunkings (1/3/7-byte feeds, chunks ending on the marker and inside the separator, every single and adjacent split offset) plus an end-to-end check that a streamed export.zip stays a parseable archive. --- .../__tests__/stream-to-disk.test.ts | 315 ++++++++++++++++++ src/lib/multipart/stream-to-disk.ts | 90 +++-- 2 files changed, 377 insertions(+), 28 deletions(-) diff --git a/src/lib/multipart/__tests__/stream-to-disk.test.ts b/src/lib/multipart/__tests__/stream-to-disk.test.ts index 33b447b90..9cc3bf973 100644 --- a/src/lib/multipart/__tests__/stream-to-disk.test.ts +++ b/src/lib/multipart/__tests__/stream-to-disk.test.ts @@ -1,10 +1,13 @@ import { describe, expect, it } from "vitest"; import { readFileSync, unlinkSync } from "node:fs"; +import { createHash } from "node:crypto"; +import { deflateRawSync } from "node:zlib"; import { parseBoundary, streamMultipartToDisk, } from "../stream-to-disk"; +import { extractExportXml } from "@/lib/import/unzip-export-xml"; const BOUNDARY = "----HealthLogTestBoundary"; @@ -142,3 +145,315 @@ describe("streamMultipartToDisk", () => { ).rejects.toThrow(/boundary/); }); }); + +/** + * Deterministic pseudo-random binary that contains the byte sequences a + * multipart parser is most likely to mis-handle: bare CRLF (0x0D 0x0A), + * "--" runs (0x2D 0x2D), and partial boundary-marker prefixes. None of + * these is the full boundary marker (the sender guarantees the marker + * cannot appear in the content), so a correct parser must copy every + * byte through verbatim. + */ +function makeAdversarialBinary(n: number): Buffer { + const b = Buffer.alloc(n); + let s = 0x12345678 >>> 0; + for (let i = 0; i < n; i++) { + s = (s * 1664525 + 1013904223) >>> 0; + b[i] = (s >>> 16) & 0xff; + } + const partialMarker = Buffer.from(`\r\n--${BOUNDARY.slice(0, 12)}`); + for (let off = 100; off + partialMarker.length < n; off += 991) { + partialMarker.copy(b, off); + } + for (let off = 37; off + 1 < n; off += 257) { + b[off] = 0x0d; + b[off + 1] = 0x0a; + } + return b; +} + +/** Build a single-entry deflate ZIP wrapping the supplied member. */ +function buildZip(name: string, data: Buffer): Buffer { + const nameBuf = Buffer.from(name, "utf8"); + const compressed = deflateRawSync(data); + const table: number[] = []; + for (let n = 0; n < 256; n++) { + let c = n; + for (let k = 0; k < 8; k++) c = (c & 1) ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + table[n] = c >>> 0; + } + let crc = 0 ^ -1; + for (let i = 0; i < data.length; i++) { + crc = (crc >>> 8) ^ table[(crc ^ data[i]) & 0xff]; + } + const crc32 = (crc ^ -1) >>> 0; + + const local = Buffer.alloc(30); + local.writeUInt32LE(0x04034b50, 0); + local.writeUInt16LE(20, 4); + local.writeUInt16LE(8, 8); + local.writeUInt32LE(crc32, 14); + local.writeUInt32LE(compressed.length, 18); + local.writeUInt32LE(data.length, 22); + local.writeUInt16LE(nameBuf.length, 26); + + const cdh = Buffer.alloc(46); + cdh.writeUInt32LE(0x02014b50, 0); + cdh.writeUInt16LE(20, 4); + cdh.writeUInt16LE(20, 6); + cdh.writeUInt16LE(8, 10); + cdh.writeUInt32LE(crc32, 16); + cdh.writeUInt32LE(compressed.length, 20); + cdh.writeUInt32LE(data.length, 24); + cdh.writeUInt16LE(nameBuf.length, 28); + + const localFull = Buffer.concat([local, nameBuf, compressed]); + const cdhFull = Buffer.concat([cdh, nameBuf]); + + const eocd = Buffer.alloc(22); + eocd.writeUInt32LE(0x06054b50, 0); + eocd.writeUInt16LE(1, 8); + eocd.writeUInt16LE(1, 10); + eocd.writeUInt32LE(cdhFull.length, 12); + eocd.writeUInt32LE(localFull.length, 16); + + return Buffer.concat([localFull, cdhFull, eocd]); +} + +/** Feed `buf` to a ReadableStream in the repeating chunk sizes given. */ +function fixedChunkStream( + buf: Buffer, + sizes: number[], +): ReadableStream { + let i = 0; + let si = 0; + return new ReadableStream({ + pull(controller) { + if (i >= buf.length) { + controller.close(); + return; + } + const size = sizes[si % sizes.length]; + si++; + const end = Math.min(buf.length, i + size); + controller.enqueue(new Uint8Array(buf.subarray(i, end))); + i = end; + }, + }); +} + +/** Split `buf` at the explicit byte offsets given. */ +function splitAtStream( + buf: Buffer, + points: number[], +): ReadableStream { + const segments: Buffer[] = []; + let prev = 0; + for (const p of points) { + segments.push(buf.subarray(prev, p)); + prev = p; + } + segments.push(buf.subarray(prev)); + let i = 0; + return new ReadableStream({ + pull(controller) { + if (i >= segments.length) { + controller.close(); + return; + } + controller.enqueue(new Uint8Array(segments[i])); + i++; + }, + }); +} + +describe("streamMultipartToDisk — large multi-chunk binary round-trip", () => { + // Regression for the cross-chunk boundary-suffix loss: a chunk that + // ended exactly on the opening boundary marker dropped the parser back + // into a fresh marker search and swallowed the whole part body, so a + // multi-chunk upload landed on disk truncated. The downstream ZIP + // reader then failed with "Could not locate ZIP End-Of-Central-Directory + // record". A tiny single-chunk upload never tripped it. + it("writes a multi-MB binary byte-exact under coarse chunkings", async () => { + // A few-MB payload exercises many chunks at realistic network sizes — + // this is the shape the wild 40 MB upload took. + const fileContent = makeAdversarialBinary(3 * 1024 * 1024 + 777); + const expectedSha = createHash("sha256") + .update(fileContent) + .digest("hex"); + const body = buildMultipartBody("export.zip", fileContent, { + userId: "u-1", + }); + + const markerLen = `--${BOUNDARY}`.length; + const sepLen = `\r\n--${BOUNDARY}`.length; + const chunkings: number[][] = [ + [body.length], // single chunk (the trivially-passing case) + [4096], + [16384, markerLen, 1, 2, 3], // realistic chunks then tiny tails + // A repeating size aligned so chunks routinely end on / inside the + // boundary marker — the exact window the bug lived in. + [markerLen, 8192], + [sepLen - 1, 8192], // ends one byte short of the full separator + [1024, 64, 5000, 13, 8192], // odd repeating mix + ]; + + for (const sizes of chunkings) { + const result = await streamMultipartToDisk( + fixedChunkStream(body, sizes), + `multipart/form-data; boundary=${BOUNDARY}`, + { maxBytes: 1.5 * 1024 * 1024 * 1024, fieldName: "file" }, + ); + try { + expect(result.bytes, `bytes for chunking ${sizes}`).toBe( + fileContent.length, + ); + expect(result.sha256, `sha for chunking ${sizes}`).toBe(expectedSha); + const written = readFileSync(result.filePath); + expect( + written.equals(fileContent), + `byte-exact for chunking ${sizes}`, + ).toBe(true); + expect(result.textFields.userId).toBe("u-1"); + } finally { + try { + unlinkSync(result.filePath); + } catch { + /* ignore */ + } + } + } + }, 20_000); + + it("writes byte-exact under fine (1/3/7-byte) chunkings", async () => { + // The pathological 1-byte feed is the cleanest reproduction of the + // boundary-suffix-loss bug; keep the payload small so the per-chunk + // file writes stay fast while still spanning thousands of chunks. + const fileContent = makeAdversarialBinary(16 * 1024 + 17); + const expectedSha = createHash("sha256") + .update(fileContent) + .digest("hex"); + const body = buildMultipartBody("export.zip", fileContent, { + userId: "u-1", + }); + + for (const sizes of [[1], [3], [7], [1, 3, 7, 2, 5, 13]]) { + const result = await streamMultipartToDisk( + fixedChunkStream(body, sizes), + `multipart/form-data; boundary=${BOUNDARY}`, + { maxBytes: 1024 * 1024, fieldName: "file" }, + ); + try { + expect(result.bytes, `bytes for chunking ${sizes}`).toBe( + fileContent.length, + ); + expect(result.sha256, `sha for chunking ${sizes}`).toBe(expectedSha); + const written = readFileSync(result.filePath); + expect( + written.equals(fileContent), + `byte-exact for chunking ${sizes}`, + ).toBe(true); + } finally { + try { + unlinkSync(result.filePath); + } catch { + /* ignore */ + } + } + } + }, 20_000); + + it("survives a chunk split at every offset (single + adjacent)", async () => { + // Small body, exhaustive split coverage — the failing case in the + // wild was a chunk boundary landing in a specific window; assert no + // offset can corrupt the captured bytes. + const fileContent = Buffer.concat([ + Buffer.from("AAAA\r\n--BBBB"), + Buffer.from(`\r\n--${BOUNDARY.slice(0, 12)}`), + Buffer.from("CCCC\r\nDDDD\r\n--"), + Buffer.from([0x0d, 0x0a, 0x2d, 0x2d, 0x00, 0xff]), + ]); + const expectedSha = createHash("sha256") + .update(fileContent) + .digest("hex"); + const body = buildMultipartBody("export.zip", fileContent, { + userId: "u-1", + }); + + for (let p = 1; p < body.length; p++) { + for (const points of [[p], [p, Math.min(p + 1, body.length)]]) { + const result = await streamMultipartToDisk( + splitAtStream(body, points), + `multipart/form-data; boundary=${BOUNDARY}`, + { maxBytes: 1024, fieldName: "file" }, + ); + try { + expect(result.sha256, `sha at split ${points}`).toBe(expectedSha); + const written = readFileSync(result.filePath); + expect( + written.equals(fileContent), + `byte-exact at split ${points}`, + ).toBe(true); + } finally { + try { + unlinkSync(result.filePath); + } catch { + /* ignore */ + } + } + } + } + }); + + it("a streamed export.zip stays a valid archive end-to-end", async () => { + // Build a real deflate ZIP whose member contains binary-flavoured + // XML, stream it through awkward chunkings, and confirm the file on + // disk is still extractable — i.e. the EOCD record survives. + let xml = `\n\n`; + let s = 42 >>> 0; + for (let i = 0; i < 8000; i++) { + s = (s * 1103515245 + 12345) >>> 0; + xml += `\n`; + } + xml += `\n`; + const xmlBuf = Buffer.from(xml, "utf8"); + const zip = buildZip("apple_health_export/export.xml", xmlBuf); + const expectedSha = createHash("sha256").update(zip).digest("hex"); + const body = buildMultipartBody("export.zip", zip, { userId: "u-1" }); + + const markerLen = `--${BOUNDARY}`.length; + for (const sizes of [[7], [markerLen, 257], [1, 3, 7, 64, 13, 257]]) { + const result = await streamMultipartToDisk( + fixedChunkStream(body, sizes), + `multipart/form-data; boundary=${BOUNDARY}`, + { maxBytes: 1.5 * 1024 * 1024 * 1024, fieldName: "file" }, + ); + let xmlPath: string | null = null; + try { + expect(result.sha256, `zip sha for chunking ${sizes}`).toBe( + expectedSha, + ); + const written = readFileSync(result.filePath); + expect(written.equals(zip), `zip byte-exact for ${sizes}`).toBe(true); + // The load-bearing assertion: the saved file is still a parseable + // archive (EOCD intact) and the member round-trips. + const extracted = extractExportXml(result.filePath); + xmlPath = extracted.xmlPath; + expect(readFileSync(extracted.xmlPath).equals(xmlBuf)).toBe(true); + } finally { + try { + unlinkSync(result.filePath); + } catch { + /* ignore */ + } + if (xmlPath) { + try { + unlinkSync(xmlPath); + } catch { + /* ignore */ + } + } + } + } + }, 20_000); +}); diff --git a/src/lib/multipart/stream-to-disk.ts b/src/lib/multipart/stream-to-disk.ts index 9e37f46ca..84a28911b 100644 --- a/src/lib/multipart/stream-to-disk.ts +++ b/src/lib/multipart/stream-to-disk.ts @@ -103,12 +103,21 @@ export async function streamMultipartToDisk( // boundary length so we never accumulate more than necessary. let buffer = Buffer.alloc(0); // Current parser state: - // "preamble" — before the first boundary - // "headers" — inside a part's header block - // "body-file" — inside the captured file's body - // "body-text" — inside a text field's body - // "epilogue" — after the closing boundary - type ParserState = "preamble" | "headers" | "body-file" | "body-text" | "epilogue"; + // "preamble" — before the first boundary marker + // "after-boundary" — a boundary marker was just consumed; awaiting + // its two-byte suffix ("\r\n" for another part or + // "--" for the closing boundary) + // "headers" — inside a part's header block + // "body-file" — inside the captured file's body + // "body-text" — inside a text field's body + // "epilogue" — after the closing boundary + type ParserState = + | "preamble" + | "after-boundary" + | "headers" + | "body-file" + | "body-text" + | "epilogue"; let state: ParserState = "preamble"; let currentFieldName: string | null = null; let currentTextBuffer: Buffer[] = []; @@ -151,19 +160,42 @@ export async function streamMultipartToDisk( // search for the boundary marker, emit bytes up to it. const processBuffer = async (): Promise => { while (true) { + if (state === "epilogue") { + // Everything after the closing boundary is discarded. + buffer = Buffer.alloc(0); + return; + } + if (state === "preamble") { const idx = buffer.indexOf(boundaryMarker); if (idx === -1) { - // Need more bytes to find a boundary; retain a small tail - // so a boundary split across chunks is still detectable. - if (buffer.length > boundaryMarker.length) { - buffer = buffer.slice(buffer.length - boundaryMarker.length); + // Need more bytes to find a boundary; retain a tail of + // `boundaryMarker.length - 1` bytes so a marker split across + // chunks is still detectable on the next read. Retaining the + // full marker length could drop a byte that completes the + // marker, so keep one fewer. + const keep = Math.min(buffer.length, boundaryMarker.length - 1); + if (buffer.length > keep) { + buffer = buffer.slice(buffer.length - keep); } return; } - // Drop everything up to and including the boundary marker. + // Drop everything up to and including the boundary marker and + // hand off to "after-boundary" to classify the two-byte suffix. + // Doing this in its own state means a chunk boundary landing + // immediately after the marker cannot reset us into a fresh + // marker search (which would silently swallow the part body). buffer = buffer.slice(idx + boundaryMarker.length); - // After the first boundary we expect "\r\n" or "--" (closing). + state = "after-boundary"; + continue; + } + + if (state === "after-boundary") { + // A boundary marker was just consumed. Wait until we have the + // two-byte suffix before deciding: "\r\n" introduces another + // part, "--" closes the multipart body. Never search for a new + // marker here — that is the bug class this state exists to + // prevent. if (buffer.length < 2) return; if (buffer[0] === 0x2d && buffer[1] === 0x2d) { state = "epilogue"; @@ -174,7 +206,20 @@ export async function streamMultipartToDisk( state = "headers"; continue; } - return; + // RFC 2046 allows linear whitespace ("transport padding") + // between the marker and its CRLF. Skip a leading run of SP/HT + // and re-test; if neither suffix is present yet, the chunk is + // malformed. + if (buffer[0] === 0x20 || buffer[0] === 0x09) { + let i = 0; + while (i < buffer.length && (buffer[i] === 0x20 || buffer[i] === 0x09)) { + i++; + } + if (i >= buffer.length) return; // need more bytes + buffer = buffer.slice(i); + continue; + } + throw new Error("Malformed multipart boundary suffix"); } if (state === "headers") { @@ -243,21 +288,10 @@ export async function streamMultipartToDisk( await closeFileSink(); } buffer = buffer.slice(idx + sep.length); - // After the boundary we expect "\r\n" or "--". - if (buffer.length < 2) { - state = "preamble"; - return; - } - if (buffer[0] === 0x2d && buffer[1] === 0x2d) { - state = "epilogue"; - return; - } - if (buffer[0] === 0x0d && buffer[1] === 0x0a) { - buffer = buffer.slice(2); - state = "headers"; - continue; - } - return; + // The boundary marker is consumed; classify its two-byte suffix + // in "after-boundary" so a chunk split landing right here cannot + // lose the next part. + state = "after-boundary"; } }; From 059c04158626f8358d78a27fcc815a4e3e6392bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 7 Jun 2026 11:25:28 +0200 Subject: [PATCH 2/2] =?UTF-8?q?chore(release):=20v1.15.8.1=20=E2=80=94=20l?= =?UTF-8?q?arge=20Apple=20Health=20imports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 6 ++++++ docs/api/openapi.yaml | 2 +- package.json | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6c1583d7..7a354c1e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +## [1.15.8.1] — 2026-06-07 — large Apple Health imports + +### Fixed + +- **Large Apple Health `export.zip` uploads no longer fail with "Could not locate ZIP End-Of-Central-Directory record".** The multipart upload reader could drop part of a file body when a network chunk ended exactly on the multipart boundary — which only showed up on larger, multi-chunk uploads (a small export imported fine). The reader now handles the boundary across chunks correctly, so the uploaded archive arrives byte-for-byte intact. (#281) + ## [1.15.8] — 2026-06-07 — medication cards ### Changed diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 513615cee..c3073b2a3 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.15.8 + version: 1.15.8.1 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. diff --git a/package.json b/package.json index e98c2d64a..34477889c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.15.8", + "version": "1.15.8.1", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "AGPL-3.0-only", "homepage": "https://healthlog.dev",