From f5bf3d115cd118c2e5ad6f46fafc5774ac875807 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:21:53 -0400 Subject: [PATCH] test(storage): cover generic-S3 public-URL resolution / forcePathStyle (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited the generic-S3 path: upload/thumbnail/.torrent, CORS guidance, and the delete/edit cleanup path-style (incl. the documented vhost bug) were already tested. The real gap was public-URL resolution — the upload tests mock buildS3ObjectUrl, so the actual path-style/vhost logic in js/storage/s3-url.js was unverified. Added tests/storage/s3-url.test.mjs (11 pure-function scenarios): path-style puts the bucket in the PATH; vhost puts it in the host subdomain (no double-prefix); endpoint path-prefix preserved; bare-endpoint https upgrade; key/slash normalization; explicit publicBaseUrl wins; missing inputs → "". Also covers s3Service.derivePublicBaseUrl / buildS3ObjectUrl. Closes #6. Co-Authored-By: Claude Opus 4.8 --- tests/storage/s3-url.test.mjs | 138 +++++++++++++++++++++++++++++ todo/TODO_2026-06-20_pre-launch.md | 23 ++++- 2 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 tests/storage/s3-url.test.mjs diff --git a/tests/storage/s3-url.test.mjs b/tests/storage/s3-url.test.mjs new file mode 100644 index 00000000..3843ea87 --- /dev/null +++ b/tests/storage/s3-url.test.mjs @@ -0,0 +1,138 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + buildS3PublicUrl, + buildS3UrlFromBase, + normalizeS3PublicBaseUrl, +} from "../../js/storage/s3-url.js"; +import { + derivePublicBaseUrl, + buildS3ObjectUrl, +} from "../../js/services/s3Service.js"; + +// #6: the generic-S3 path's public-URL resolution must honor forcePathStyle. +// Upload tests mock buildS3ObjectUrl, so the real path-style/vhost logic in +// s3-url.js is asserted here directly (pure functions, no mocking). + +const ENDPOINT = "https://s3.example.com"; +const BUCKET = "mybucket"; +const KEY = "u/npub1abc/hash/clip.mp4"; + +test("forcePathStyle=true puts the bucket in the PATH (generic S3 / MinIO style)", () => { + const url = buildS3PublicUrl({ + endpoint: ENDPOINT, + bucket: BUCKET, + key: KEY, + forcePathStyle: true, + }); + assert.equal(url, `https://s3.example.com/mybucket/${KEY}`); +}); + +test("forcePathStyle=false puts the bucket in the HOST as a subdomain (virtual-hosted style)", () => { + const url = buildS3PublicUrl({ + endpoint: ENDPOINT, + bucket: BUCKET, + key: KEY, + forcePathStyle: false, + }); + assert.equal(url, `https://mybucket.s3.example.com/${KEY}`); +}); + +test("vhost style does not double-prefix a bucket already present in the hostname", () => { + const url = buildS3PublicUrl({ + endpoint: "https://mybucket.s3.example.com", + bucket: BUCKET, + key: KEY, + forcePathStyle: false, + }); + assert.equal(url, `https://mybucket.s3.example.com/${KEY}`); +}); + +test("path-style preserves an endpoint path prefix (e.g. self-hosted under /v1)", () => { + const url = buildS3PublicUrl({ + endpoint: "https://host.internal/v1", + bucket: BUCKET, + key: KEY, + forcePathStyle: true, + }); + assert.equal(url, `https://host.internal/v1/mybucket/${KEY}`); +}); + +test("a bare endpoint (no scheme) is upgraded to https and trailing slashes are trimmed", () => { + const url = buildS3PublicUrl({ + endpoint: "s3.example.com/", + bucket: BUCKET, + key: "k.mp4", + forcePathStyle: true, + }); + assert.equal(url, "https://s3.example.com/mybucket/k.mp4"); +}); + +test("leading slashes on the key are normalized away (no //)", () => { + const url = buildS3PublicUrl({ + endpoint: ENDPOINT, + bucket: BUCKET, + key: "///u/clip.mp4", + forcePathStyle: true, + }); + assert.equal(url, "https://s3.example.com/mybucket/u/clip.mp4"); +}); + +test("missing endpoint or bucket yields an empty string (no malformed URL)", () => { + assert.equal(buildS3PublicUrl({ bucket: BUCKET, key: KEY, forcePathStyle: true }), ""); + assert.equal(buildS3PublicUrl({ endpoint: ENDPOINT, key: KEY, forcePathStyle: true }), ""); +}); + +test("buildS3UrlFromBase joins a custom public base URL with the key", () => { + assert.equal( + buildS3UrlFromBase({ publicBaseUrl: "https://cdn.example.com", key: KEY }), + `https://cdn.example.com/${KEY}`, + ); + // Trailing slash on the base is handled. + assert.equal( + buildS3UrlFromBase({ publicBaseUrl: "https://cdn.example.com/", key: "a/b.mp4" }), + "https://cdn.example.com/a/b.mp4", + ); + // Missing pieces → empty. + assert.equal(buildS3UrlFromBase({ publicBaseUrl: "", key: KEY }), ""); + assert.equal(buildS3UrlFromBase({ publicBaseUrl: "https://cdn.example.com", key: "" }), ""); +}); + +test("normalizeS3PublicBaseUrl trims trailing slashes and whitespace", () => { + assert.equal(normalizeS3PublicBaseUrl(" https://cdn.example.com/// "), "https://cdn.example.com"); + assert.equal(normalizeS3PublicBaseUrl(""), ""); +}); + +test("s3Service.derivePublicBaseUrl returns the bucket root honoring forcePathStyle", () => { + assert.equal( + derivePublicBaseUrl({ endpoint: ENDPOINT, bucket: BUCKET, forcePathStyle: true }), + "https://s3.example.com/mybucket", + ); + assert.equal( + derivePublicBaseUrl({ endpoint: ENDPOINT, bucket: BUCKET, forcePathStyle: false }), + "https://mybucket.s3.example.com", + ); +}); + +test("s3Service.buildS3ObjectUrl prefers an explicit publicBaseUrl over endpoint derivation", () => { + // When a custom CDN/public base is configured, it wins — even if endpoint/bucket + // would derive something different. + const url = buildS3ObjectUrl({ + publicBaseUrl: "https://cdn.example.com", + endpoint: ENDPOINT, + bucket: BUCKET, + key: KEY, + forcePathStyle: true, + }); + assert.equal(url, `https://cdn.example.com/${KEY}`); + + // Without a publicBaseUrl it falls back to endpoint derivation (path-style here). + const derived = buildS3ObjectUrl({ + endpoint: ENDPOINT, + bucket: BUCKET, + key: KEY, + forcePathStyle: true, + }); + assert.equal(derived, `https://s3.example.com/mybucket/${KEY}`); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index b67b1863..174868b4 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -420,10 +420,25 @@ toward freshness and looked identical. Gave each a structural identity: ## Open — medium priority -### 6. Test S3 (generic, non-R2) functions -- [ ] End-to-end test the generic-S3 path (not just Cloudflare R2): upload, thumbnail, - `.torrent`, public-base-URL resolution, CORS guidance, and **delete/edit cleanup** - (provider-aware path-style). Confirm `forcePathStyle` is honored throughout. +### 6. Test S3 (generic, non-R2) functions — DONE 2026-06-24 +- [x] **Audited; was more covered than it looked.** Already tested: upload/thumbnail/ + `.torrent` (`s3UploadService.test.mjs`, `s3-upload-*`), CORS guidance + (`s3-upload-cors-guidance.test.mjs`), and **delete/edit cleanup provider-aware + path-style** — incl. the documented vhost bug (`r2-delete-video-storage.test.mjs` + asserts cleanup uses `forcePathStyle:false` for generic S3; + `s3-upload-publish-cleanup-integration.test.mjs` covers the full upload→delete + round-trip). `forcePathStyle` is plumbed through upload + delete. +- [x] **Closed the real gap: public-URL resolution.** The upload tests *mock* + `buildS3ObjectUrl`, so the actual path-style/vhost URL logic in `js/storage/ + s3-url.js` had no focused test. Added `tests/storage/s3-url.test.mjs` (11 pure- + function scenarios): path-style → bucket in PATH; vhost → bucket as host + subdomain (no double-prefix); endpoint path-prefix preserved; bare-endpoint + https upgrade; key/slash normalization; explicit `publicBaseUrl` wins; missing + inputs → "". Also covers `s3Service.derivePublicBaseUrl`/`buildS3ObjectUrl`. +- [~] Provider forcePathStyle DEFAULTS (generic→path-style true, aws→vhost false) are + a 3-line internal config map (`resolveForcePathStyle`/`PROVIDER_TESTS`, not + exported); not unit-tested in isolation (would require exposing internals). + Behavior is exercised via the upload/delete tests above. Acceptable minor gap. ### 7. Mobile + video-card layout - [ ] Improve mobile layout and the video card layout.