mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
[ui] upload: route by the credentials' own provider (fix "S3 endpoint is required")
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 <[email protected]>
This commit is contained in:
1 parent
f00d2b5f49
commit
5afd602d52
3 files changed
+98
-7
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user