Skip to content

Commit ead684c

Browse files
authored
Merge pull request #63 from cohm/d3-deps-and-pdf-hardening
Depend on the three d3 modules we use, and stop the PDF route rendering hostile HTML
2 parents 02327cf + c221c1d commit ead684c

6 files changed

Lines changed: 155 additions & 555 deletions

File tree

CLAUDE.md

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,14 +536,59 @@ unconditionally so it shrinks as well as grows.
536536

537537
**Vercel PDF export**: `vercel.json` sets 1800 MB RAM and 60s timeout for the PDF endpoint (Hobby plan limit is 2048 MB). The `@sparticuz/chromium` binary must be bundled — configured via `serverExternalPackages` + `outputFileTracingIncludes` in `next.config.ts` and the matching `includeFiles` in `vercel.json` (the duplication is intentional but fragile).
538538

539+
**`import 'd3-transition'` is load-bearing and must not be tidied away.** It is a
540+
side-effect import: it is what installs `.transition()` and `.interrupt()` on the
541+
selection prototype, and `TimelineVisualization` calls `.interrupt()` 17 times.
542+
Measured: `selection.prototype.interrupt` is `undefined` after importing only
543+
`d3-selection` and a function after importing `d3-transition`. **Neither `tsc` nor
544+
`eslint` catches its absence**, because `@types/d3-selection`/`@types/d3` declare
545+
the augmentation regardless of which module provides it — so removing it
546+
type-checks, lints, and then throws at runtime on all 17 call sites.
547+
548+
Related, and deliberately left alone: nothing in the codebase calls
549+
`.transition()`, so those 17 `.interrupt()` calls are currently no-ops. Dropping
550+
them would take the d3 install tree from 13 packages to 9, but that is a
551+
behaviour decision rather than a packaging one.
552+
553+
**Why submodule imports at all**, given `import * as d3` was already tree-shaken
554+
(verified by probing built chunks for minification-surviving string literals:
555+
`__data__` and `getUTCMonth` present, `MultiPolygon`, `Invalid delimiter` and
556+
`d8b365` absent; bundle 368.68 → 368.71 kB, i.e. unchanged). The win is the
557+
install tree: **38 packages reachable from `d3` with 7 non-d3 transitive
558+
dependencies, down to 13 with 1** (`internmap`). Four of the dropped ones —
559+
`commander`, `iconv-lite`, `rw`, `safer-buffer` — arrived via `d3-dsv`, a CSV
560+
parser that ships a command-line tool this app never calls.
561+
562+
**The PDF endpoint renders caller-supplied HTML, so it is locked down.**
563+
`/api/export-pdf` takes an `html` string and renders it in headless Chrome, which
564+
without guards is a rendering oracle: submitted HTML can `fetch()` an address
565+
reachable from the function and write the response into the DOM, where it returns
566+
inside the PDF. Three guards, all inert for a real export because the document we
567+
generate is entirely static (inline `<style>`, base64 `@font-face` data URIs, a
568+
serialised SVG, no `<script>`, no external URL — the Google Fonts fetch happens in
569+
the browser *before* the POST):
570+
571+
- same-origin only (`Origin`, falling back to `Referer`, compared against `Host`);
572+
- `page.setJavaScriptEnabled(false)`;
573+
- request interception allowing only `data:`, `about:` and `blob:`.
574+
575+
`setContent` is implemented via page-context evaluation, so disabling scripts
576+
could plausibly have broken the feature. Verified against real Chrome that it does
577+
not: our export document renders to a **byte-identical** PDF with and without the
578+
guards. A document carrying a script and an `<img>` at `169.254.169.254` hung
579+
until the 30 s navigation timeout unguarded, and rendered promptly with the script
580+
not run and the request blocked when guarded. One deployment caveat: the check
581+
compares against the `Host` header, so a proxy that rewrites `Host` would reject
582+
exports.
583+
539584
**Build-time metadata**: `next.config.ts` injects `NEXT_PUBLIC_GIT_HASH`, `NEXT_PUBLIC_GIT_TIMESTAMP`, and `NEXT_PUBLIC_GIT_REPO_URL` at build time, falling back to `git` shell-outs when the Vercel env vars aren't present.
540585

541586
**Standing review**: Open issues, design discussion, and a ranked improvement list live in `REVIEW.md` at the repo root.
542587

543588
## Tech Stack
544589

545590
- **Next.js 16** (App Router), **React 19** with React Compiler, **TypeScript 5** (strict)
546-
- **D3.js 7** for all SVG rendering
591+
- **D3 7** for all SVG rendering — imported as the three submodules actually used (`d3-selection`, `d3-scale`, `d3-color`) rather than the `d3` meta-package, plus a side-effect import of `d3-transition` (see below)
547592
- **Tailwind CSS 4**
548593
- **Puppeteer-core + @sparticuz/chromium** for server-side PDF
549594

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ Troubleshooting
124124
Key production dependencies:
125125
- `next` (16.x) — React framework
126126
- `react` (19.x) — UI library
127-
- `d3` (7.9.x) — Visualization and data manipulation
127+
- `d3-selection`, `d3-scale`, `d3-color` (3.x/4.x) — the three D3 modules used for SVG rendering, imported directly rather than via the `d3` meta-package
128+
- `d3-transition` (3.x) — imported for its side effect only; it provides `.interrupt()` on selections
128129
- `puppeteer-core` (25.x) — Headless browser control for PDF generation
129130
- `@sparticuz/chromium` (141.x) — Serverless-compatible Chrome binary for Vercel
130131

0 commit comments

Comments
 (0)