From 5afd602d52be395cee839f099a4dfb708c6848d8 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Tue, 30 Jun 2026 15:35:18 -0400 Subject: [PATCH] [ui] upload: route by the credentials' own provider (fix "S3 endpoint is required") MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With both Backblaze B2 and Cloudflare R2 configured, an upload could fail with "S3 endpoint is required." when the upload modal's tracked provider drifted out of sync with the active credentials (user: storage selector on Backblaze but the modal set for Cloudflare). A Cloudflare R2 connection stores an accountId and derives its endpoint, so routing it through the S3 service — which requires an explicit endpoint — threw. Fix: MediaUploader.uploadVideo/uploadThumbnail now choose the service + provider from the CREDENTIALS' own provider (credentials.provider || credentials.meta.provider || provider), so a stale modal provider can no longer send an R2 connection through the S3 path (or vice-versa). The R2 service derives the endpoint from accountId as before. Test: tests/media-uploader-provider-routing.test.mjs (4 — R2 creds route to r2Service even with a stale Backblaze arg; B2 creds route to s3Service; meta.provider + passed-arg fallbacks). Build + lint clean; upload/storage suites green. Co-Authored-By: Claude Opus 4.8 --- js/ui/components/mediaUploader.js | 22 ++++-- .../media-uploader-provider-routing.test.mjs | 71 +++++++++++++++++++ todo/TODO_2026-06-20_pre-launch.md | 12 ++++ 3 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 tests/media-uploader-provider-routing.test.mjs diff --git a/js/ui/components/mediaUploader.js b/js/ui/components/mediaUploader.js index 4feb2f1c..00dee287 100644 --- a/js/ui/components/mediaUploader.js +++ b/js/ui/components/mediaUploader.js @@ -111,7 +111,11 @@ export class MediaUploader { async uploadThumbnail(file, { provider, credentials, onProgress } = {}) { const pubkey = this.getCurrentPubkey(); const npub = this.safeEncodeNpub(pubkey); - const service = this.serviceFor(provider); + // Route by the credentials' own provider (see uploadVideo) to avoid wrong-service + // routing when the modal's provider state is stale. + const effectiveProvider = + credentials?.provider || credentials?.meta?.provider || provider; + const service = this.serviceFor(effectiveProvider); const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials, @@ -121,7 +125,7 @@ export class MediaUploader { const cleanName = file.name.replace(/[^a-zA-Z0-9.-]/g, "_"); const key = `${npub}/thumbnails/${timestamp}-${cleanName}`; const baseDomain = bucketEntry.publicBaseUrl; - const url = this.publicUrlFor(provider, baseDomain, key); + const url = this.publicUrlFor(effectiveProvider, baseDomain, key); await service.uploadFile({ file, @@ -129,7 +133,7 @@ export class MediaUploader { key, accountId: settings.accountId, endpoint: settings.endpoint, - provider: settings.provider || provider || "cloudflare_r2", + provider: settings.provider || effectiveProvider || "cloudflare_r2", region: settings.region, accessKeyId: settings.accessKeyId, secretAccessKey: settings.secretAccessKey, @@ -201,7 +205,11 @@ export class MediaUploader { const pubkey = this.getCurrentPubkey(); const npub = this.safeEncodeNpub(pubkey); - const service = this.serviceFor(provider); + // Route by the credentials' OWN provider so a stale activeProvider can't send an + // R2 connection (accountId, no endpoint) through the S3 path — or vice-versa. + const effectiveProvider = + credentials?.provider || credentials?.meta?.provider || provider; + const service = this.serviceFor(effectiveProvider); const { settings, bucketEntry } = await service.prepareUpload(npub, { credentials, @@ -209,7 +217,7 @@ export class MediaUploader { const videoKey = buildR2Key(npub, file, identifier); const baseDomain = bucketEntry.publicBaseUrl; - const videoPublicUrl = this.publicUrlFor(provider, baseDomain, videoKey); + const videoPublicUrl = this.publicUrlFor(effectiveProvider, baseDomain, videoKey); emit(0, "Uploading video..."); const uploadPromise = service.uploadFile({ @@ -218,7 +226,7 @@ export class MediaUploader { key: videoKey, accountId: settings.accountId, endpoint: settings.endpoint, - provider: settings.provider || provider || "cloudflare_r2", + provider: settings.provider || effectiveProvider || "cloudflare_r2", region: settings.region, accessKeyId: settings.accessKeyId, secretAccessKey: settings.secretAccessKey, @@ -239,7 +247,7 @@ export class MediaUploader { publicBaseUrl: baseDomain, key: videoKey, }); - const providerLabel = isR2Provider(provider) ? "r2" : "s3"; + const providerLabel = isR2Provider(effectiveProvider) ? "r2" : "s3"; const storagePointer = buildStoragePointerValue({ provider: providerLabel, prefix: storagePrefix, diff --git a/tests/media-uploader-provider-routing.test.mjs b/tests/media-uploader-provider-routing.test.mjs new file mode 100644 index 00000000..1bb70eb7 --- /dev/null +++ b/tests/media-uploader-provider-routing.test.mjs @@ -0,0 +1,71 @@ +// Bug: with a Backblaze + Cloudflare connection both configured, uploads failed with +// "S3 endpoint is required." when the upload modal's tracked provider drifted out of +// sync with the active credentials — a Cloudflare R2 connection (accountId, no endpoint) +// got routed through the S3 service. +// +// Fix: MediaUploader routes by the CREDENTIALS' own provider, not a separately-passed +// (and possibly stale) provider. These tests exercise that via uploadThumbnail (no +// torrent generation, so no webtorrent in the test). + +import test from "node:test"; +import { strict as assert } from "node:assert"; +import { MediaUploader } from "../js/ui/components/mediaUploader.js"; + +function makeUploader() { + const used = []; + const mkService = (name) => ({ + prepareUpload: async (_npub, { credentials }) => { + used.push({ service: name, credentials }); + return { + settings: { provider: credentials?.provider, accountId: "acct", endpoint: "" }, + bucketEntry: { publicBaseUrl: "https://cdn.example", bucket: "bkt" }, + }; + }, + uploadFile: async () => {}, + }); + const r2Service = mkService("r2"); + const s3Service = mkService("s3"); + const uploader = new MediaUploader({ + r2Service, + s3Service, + storageService: null, + getCurrentPubkey: () => "f".repeat(64), + safeEncodeNpub: () => "npub1abc", + }); + return { uploader, used }; +} + +const R2_CREDS = { provider: "cloudflare_r2", accountId: "acct" }; // no endpoint! +const B2_CREDS = { provider: "backblaze_b2", endpoint: "https://s3.us-west-004.backblazeb2.com" }; +const file = { name: "thumb.jpg" }; + +test("a Cloudflare connection routes to the R2 service even when the passed provider is stale (Backblaze)", async () => { + const { uploader, used } = makeUploader(); + await uploader.uploadThumbnail(file, { provider: "backblaze_b2", credentials: R2_CREDS }); + assert.equal(used.length, 1); + assert.equal(used[0].service, "r2", "routed by credentials.provider, not the stale arg"); +}); + +test("a Backblaze connection routes to the S3 service even when the passed provider is stale (Cloudflare)", async () => { + const { uploader, used } = makeUploader(); + await uploader.uploadThumbnail(file, { provider: "cloudflare_r2", credentials: B2_CREDS }); + assert.equal(used[0].service, "s3"); +}); + +test("falls back to meta.provider when the top-level provider is absent", async () => { + const { uploader, used } = makeUploader(); + await uploader.uploadThumbnail(file, { + provider: "backblaze_b2", + credentials: { meta: { provider: "cloudflare_r2" }, accountId: "acct" }, + }); + assert.equal(used[0].service, "r2"); +}); + +test("falls back to the passed provider when credentials carry none", async () => { + const { uploader, used } = makeUploader(); + await uploader.uploadThumbnail(file, { + provider: "cloudflare_r2", + credentials: { accountId: "acct" }, + }); + assert.equal(used[0].service, "r2"); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index ba2e4a04..0e7c37fc 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -937,6 +937,18 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro `tests/storage-connections.test.mjs` (7 — pure default/dup logic + integration: R2+B2 coexist with one default, re-default switch, legacy-`default` migration, and the export carries both). Storage + profile-modal suites 69/69; build + lint clean. +- [x] **Upload wrong-service routing FIXED 2026-06-30 (multi-provider regression).** With + both B2 + Cloudflare configured, an upload could fail with **"S3 endpoint is + required."** when the upload modal's tracked `activeProvider` drifted out of sync + with `activeCredentials` (user: "selector on Backblaze but the modal set for + Cloudflare") — an R2 connection (accountId, no endpoint) got routed through the S3 + service. Fix: `MediaUploader.uploadVideo`/`uploadThumbnail` now route by the + CREDENTIALS' own provider (`credentials.provider || meta.provider || provider`), so + a stale modal provider can't pick the wrong service. Test: + `tests/media-uploader-provider-routing.test.mjs` (4). Build + lint clean. +- [ ] **Follow-up (cosmetic):** the upload modal's summary provider label can still show + a stale provider until reloaded — refresh `loadFromStorage` when the storage default + changes in the Storage pane (functional routing is already correct). ### 39. Image uploads should prefer uploading to configured storage (UX) - [x] **Profile image + banner upload — FIXED 2026-06-30.** The UI was already wired