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