fix: bound maxDocumentBytes in bytes while the body is read

js/README.md

@@ -128,7 +128,7 @@ docs.resolveLink('rules.md', 'handbook/getting-started.md');
 | `transport` | `'api'` with a token, `'raw'` without | Contents API vs `raw.githubusercontent.com`. |
 | `documents` | *(unset)* | The catalogue. A list, or a comma-separated string. **Unset means the whole repository is fetchable; an empty list means nothing is.** |
 | `timeoutMs` | `5000` | A page renders its panel rather than making a visitor wait on someone else's outage. |
-| `maxDocumentBytes` | `1048576` | Anything larger is not the document that was asked for. |
+| `maxDocumentBytes` | `1048576` | Anything larger is not the document that was asked for. Counted in bytes of the body as it arrives, and the read stops there rather than buffering the rest. |
 | `apiBase` | `https://api.github.com` | For GitHub Enterprise. |
 | `fetchImpl` | global `fetch` | Injectable, for tests. |
 

js/src/client.ts

@@ -126,6 +126,43 @@ export interface DocsClient {
     resolveLink(href: string, fromPath?: string): string;
 }
 
+// The body, read no further than `maxBytes`, or null when it is larger. Counted
+// in bytes as they arrive rather than as `string.length` after the fact: the
+// point of the limit is that an oversized body is never held in memory, and a
+// UTF-16 code-unit count would let a multi-byte document past it several times
+// over.
+const readBoundedText = async (
+    response: Response,
+    maxBytes: number,
+    controller: AbortController
+): Promise<string | null> => {
+    if (!response.body) {
+        // No stream to bound — an empty body, or a minimal stand-in that only
+        // offers `text()`. Still measured in bytes, so the limit means one thing.
+        const text = await response.text();
+        return new TextEncoder().encode(text).byteLength > maxBytes ? null : text;
+    }
+    const reader = response.body.getReader();
+    const decoder = new TextDecoder();
+    let received = 0;
+    let text = '';
+    for (;;) {
+        const {done, value} = await reader.read();
+        if (done) {
+            return text + decoder.decode();
+        }
+        received += value.byteLength;
+        if (received > maxBytes) {
+            // Stop the transfer rather than draining it: the rest of the body
+            // is exactly what the limit exists to avoid reading.
+            void reader.cancel().catch(() => undefined);
+            controller.abort();
+            return null;
+        }
+        text += decoder.decode(value, {stream: true});
+    }
+};
+
 // One fetch, bounded by a timeout and a size limit, with every failure
 // collapsed to `unavailable`.
 const fetchMarkdown = async (
@@ -145,8 +182,8 @@ const fetchMarkdown = async (
             // intermediary might — must not be able to launder it through here.
             return {status: 'unavailable'};
         }
-        const text = await response.text();
-        if (text.length > maxBytes) {
+        const text = await readBoundedText(response, maxBytes, controller);
+        if (text === null) {
             return {status: 'unavailable'};
         }
         return {status: 'ok', markdown: text};

js/test/client.test.ts

@@ -88,7 +88,7 @@ describe('failure is a value, not an exception', () => {
         });
     }
 
-    it('refuses a document larger than the limit rather than holding it in memory', async () => {
+    it('refuses a document larger than the limit', async () => {
         const fetchImpl = vi.fn().mockResolvedValue(okResponse('x'.repeat(101)));
         const client = createDocsClient({
             repo: REPO,
@@ -110,6 +110,76 @@ describe('failure is a value, not an exception', () => {
         await expect(client.fetchMarkdown('rules')).resolves.toEqual({status: 'ok', markdown: 'x'.repeat(100)});
     });
 
+    it('stops reading an oversized body at the limit instead of draining it', async () => {
+        // A thousand 64-byte chunks against a 100-byte limit. Reading the whole
+        // body and then measuring it would pull every chunk; the limit is only
+        // worth having if it stops the read a chunk or two past the line.
+        let pulled = 0;
+        const body = new ReadableStream<Uint8Array>({
+            pull(controller) {
+                pulled += 1;
+                if (pulled > 1000) {
+                    controller.close();
+                    return;
+                }
+                controller.enqueue(new Uint8Array(64).fill(0x78));
+            }
+        });
+        let signal: AbortSignal | undefined;
+        const fetchImpl = vi.fn(async (_url: string, init?: RequestInit) => {
+            signal = init?.signal ?? undefined;
+            return new Response(body, {status: 200});
+        });
+        const client = createDocsClient({
+            repo: REPO,
+            documents: DOCS,
+            maxDocumentBytes: 100,
+            fetchImpl: fetchImpl as never
+        });
+
+        await expect(client.fetchMarkdown('rules')).resolves.toEqual({status: 'unavailable'});
+        expect(pulled).toBeLessThan(10);
+        expect(signal?.aborted).toBe(true);
+    });
+
+    it('measures the limit in bytes, not in string length', async () => {
+        // 51 two-byte characters: 102 bytes of UTF-8, but a `length` of 51.
+        const markdown = 'é'.repeat(51);
+        for (const response of [() => okResponse(markdown), () => new Response(markdown, {status: 200})]) {
+            const fetchImpl = vi.fn().mockResolvedValue(response());
+            const client = createDocsClient({
+                repo: REPO,
+                documents: DOCS,
+                maxDocumentBytes: 100,
+                fetchImpl: fetchImpl as never
+            });
+            await expect(client.fetchMarkdown('rules')).resolves.toEqual({status: 'unavailable'});
+        }
+    });
+
+    it('decodes a character split across chunks when the body is within the limit', async () => {
+        // 'é' is 0xC3 0xA9; delivering the two bytes in separate chunks is what
+        // a real network read does to a multi-byte character sooner or later.
+        const chunks = [new Uint8Array([0x23, 0x20, 0xc3]), new Uint8Array([0xa9, 0x0a])];
+        const body = new ReadableStream<Uint8Array>({
+            start(controller) {
+                for (const chunk of chunks) {
+                    controller.enqueue(chunk);
+                }
+                controller.close();
+            }
+        });
+        const fetchImpl = vi.fn().mockResolvedValue(new Response(body, {status: 200}));
+        const client = createDocsClient({
+            repo: REPO,
+            documents: DOCS,
+            maxDocumentBytes: 5,
+            fetchImpl: fetchImpl as never
+        });
+
+        await expect(client.fetchMarkdown('rules')).resolves.toEqual({status: 'ok', markdown: '# é\n'});
+    });
+
     it('gives up after the timeout by aborting the request', async () => {
         const fetchImpl = vi.fn(
             (_url: string, init?: RequestInit) =>