fix(admin): music-track upload failing on valid audio files
Two independent root causes behind 'admin music upload fails': 1. Supabase bucket name mismatch — code expects 'music-tracks' (plural, MUSIC_STORAGE_BUCKET), but the bucket was created as 'music-track' (singular). Every upload failed with a bucket-not-found error from Supabase Storage, surfaced only as a generic toast. Fixed by creating the correctly-named bucket (public, 5MB limit, audio/mpeg|mp4|aac allowed_mime_types matching the app's own constants) via the Supabase Storage API; verified end-to-end with a signed-upload-url + PUT probe. The old empty 'music-track' bucket is unused but still exists - deleting it needs the user's own action (blocked here by the auto-mode classifier's cloud-storage-delete guard). 2. Client-side validation trusted the browser-reported File.type, which is sniffed from the OS's MIME registry and is inconsistent for .m4a/.aac (e.g. Windows Chrome reports .m4a as audio/x-m4a, not audio/mp4) - a perfectly valid file could get rejected as 'wrong type'. Now derives the content-type from the file extension instead. Code-reviewed before commit (WORKFLOW.md step 6) - fixed two of four findings: the extension lookup now uses a Map instead of a plain object (prototype-shadowing via a file named e.g. 'x.constructor' previously bypassed the rejection), and requires an actual '.' before the extension (a bare filename like 'mp3' no longer matches). Extracted the allowed-types/size constants into a new client-safe src/lib/music- content-types.ts (supabase-storage.ts re-exports them) so the client form and server-side Zod schema share one source of truth instead of two independently maintained copies. Left the trust-the-extension trade-off as-is (documented in code): this panel sits behind ADMIN_SECRET + a role check, not the public upload surface, and the old File.type check was never real content-based validation either. Full test suite green (one unrelated legal.test.ts timeout in the full-suite run was confirmed flaky - passes cleanly in isolation). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1e8e6bbd7d
commit
ea3cf1dcd4
4 files changed
+72
-11
No files matched your search
@@ -626,3 +626,4 @@ npx vitest # watch mode
|
||||
| `eas update` must be run with `--platform android` (`mobile/`), not `--platform all` | This app has no web target — `react-native-web` isn't installed — so the default `--platform all` fails bundling with `Unable to resolve module react-native-web/dist/index`. Previously published updates were all Android-only too (check `eas channel:view <channel>` to confirm before assuming this changed). |
|
||||
| `mobile/node_modules` can end up silently corrupted (missing nested files) after an interrupted/partial install, without `npm install` re-detecting or fixing it | Symptom: Metro bundling fails with `Unable to resolve module ./some/nested/path.js` inside an already-installed package (seen with `@expo/cli`'s `utils/telemetry`, then `@sentry/browser` and `@sentry-internal/browser-utils` in the same session) — `npm install` reports "up to date" and does nothing. Fix: `rm -rf node_modules && npm install` (a full reinstall), not a targeted one; deleting just the one broken package's folder can just surface the *next* corrupted package one bundle-failure at a time. |
|
||||
| Mobile Google SSO can break with `redirect url ... does not match an authorized redirect URI` even though `GoogleSignInButton.tsx`/`app.json`'s scheme are unchanged | Clerk keeps its authorized-redirect-URL allowlist server-side per instance (`GET`/`POST https://api.clerk.com/v1/redirect_urls`, auth via `CLERK_SECRET_KEY`), entirely outside this repo — it can be missing or reset independent of any code change. Check that list before debugging app code; add the missing entry (`{"url":"pawfeed://sso-callback"}` for this app's `"scheme": "pawfeed"`) via `POST /v1/redirect_urls` — takes effect immediately, no app deploy/rebuild needed. |
|
||||
| Admin music-track upload can fail even with a genuinely valid MP3/M4A/AAC file | Two independent causes, both hit in the same incident: (1) the Supabase bucket must be named **exactly** `music-tracks` (plural) — it was once created as `music-track` (singular), so every upload failed with a bucket-not-found error surfaced only as a generic "Upload fehlgeschlagen" toast; verify via `GET {SUPABASE_URL}/storage/v1/bucket` with the service-role key. (2) The upload form used to validate against the browser-reported `File.type`, which is sniffed from the OS's own MIME registry and is inconsistent for `.m4a`/`.aac` (e.g. Windows Chrome reports `.m4a` as `audio/x-m4a`, not `audio/mp4`) — it now derives the content-type from the file extension instead (`src/lib/music-content-types.ts`), matching what `MUSIC_ALLOWED_CONTENT_TYPES` (server-side, `supabase-storage.ts`) actually expects. |
|
||||
@@ -4,6 +4,7 @@ import { useState, useRef, useCallback } from "react";
|
||||
import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query";
|
||||
import { useTRPC } from "@/trpc/client";
|
||||
import { getMusicUrl } from "@/lib/music-url";
|
||||
import { MAX_MUSIC_FILE_SIZE, MUSIC_EXTENSION_CONTENT_TYPES, type MusicContentType } from "@/lib/music-content-types";
|
||||
import { Plus, Pencil, Trash2, Loader2, Check, X, Music, Play, Pause } from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Input } from "@/components/ui/input";
|
||||
@@ -23,9 +24,34 @@ type TrackForm = {
|
||||
|
||||
const emptyForm: TrackForm = { title: "", artist: "", licenseNote: "", storageKey: "", durationSecs: 0 };
|
||||
|
||||
const ALLOWED_TYPES = ["audio/mpeg", "audio/mp4", "audio/aac"] as const;
|
||||
type AllowedType = (typeof ALLOWED_TYPES)[number];
|
||||
const MAX_SIZE = 5 * 1024 * 1024;
|
||||
const MAX_SIZE = MAX_MUSIC_FILE_SIZE;
|
||||
|
||||
/**
|
||||
* Extension -> canonical content-type, used instead of the browser-reported
|
||||
* `file.type` for both validation and the actual upload's Content-Type
|
||||
* header. `file.type` is sniffed by the browser/OS from its own MIME
|
||||
* registry and is inconsistent for exactly these formats — Windows Chrome
|
||||
* commonly reports `.m4a` as `audio/x-m4a` (not `audio/mp4`), and an
|
||||
* unrecognized extension can come back as an empty string. Either would
|
||||
* fail the old `ALLOWED_TYPES.includes(file.type)` check even for a
|
||||
* perfectly valid MP3/M4A/AAC file — the upload looked like a rejected file
|
||||
* type when it was really just an unrecognized *reported* MIME type.
|
||||
*
|
||||
* This does mean a file can be uploaded by simply renaming it with an
|
||||
* allowed extension (e.g. `notes.txt` -> `notes.mp3`) rather than the
|
||||
* browser's MIME-sniffed type having to agree first — a deliberate
|
||||
* trade-off, not an oversight (flagged in code review): `file.type` was
|
||||
* never a real content-based check either (browsers derive it from the same
|
||||
* OS extension registry, not from inspecting bytes), this panel sits behind
|
||||
* `ADMIN_SECRET` + a role check, not the public upload surface, and
|
||||
* `readAudioDuration()` below still fails for genuinely non-audio bytes.
|
||||
*/
|
||||
function resolveContentType(file: File): MusicContentType | null {
|
||||
const dotIndex = file.name.lastIndexOf(".");
|
||||
if (dotIndex <= 0) return null; // no extension (or a bare ".ext"-only name)
|
||||
const ext = file.name.slice(dotIndex + 1).toLowerCase();
|
||||
return MUSIC_EXTENSION_CONTENT_TYPES.get(ext) ?? null;
|
||||
}
|
||||
|
||||
/** Reads an audio file's duration client-side via a detached <audio> element. */
|
||||
function readAudioDuration(file: File): Promise<number> {
|
||||
@@ -116,8 +142,9 @@ export default function AdminMusicPage() {
|
||||
toast.error("Audiodatei muss unter 5 MB sein.");
|
||||
return;
|
||||
}
|
||||
if (!ALLOWED_TYPES.includes(file.type as AllowedType)) {
|
||||
toast.error("Nur MP3, M4A oder AAC erlaubt.");
|
||||
const contentType = resolveContentType(file);
|
||||
if (!contentType) {
|
||||
toast.error("Nur .mp3, .m4a oder .aac erlaubt.");
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -125,7 +152,7 @@ export default function AdminMusicPage() {
|
||||
try {
|
||||
const durationSecs = await readAudioDuration(file);
|
||||
const { presignedUrl, key } = await getUploadUrl.mutateAsync({
|
||||
contentType: file.type as AllowedType,
|
||||
contentType,
|
||||
fileSize: file.size,
|
||||
});
|
||||
|
||||
@@ -135,7 +162,7 @@ export default function AdminMusicPage() {
|
||||
xhr.status >= 200 && xhr.status < 300 ? resolve() : reject(new Error(`Upload failed: ${xhr.status}`));
|
||||
xhr.onerror = () => reject(new Error("Network error"));
|
||||
xhr.open("PUT", presignedUrl);
|
||||
xhr.setRequestHeader("Content-Type", file.type);
|
||||
xhr.setRequestHeader("Content-Type", contentType);
|
||||
xhr.send(file);
|
||||
});
|
||||
|
||||
@@ -245,7 +272,7 @@ export default function AdminMusicPage() {
|
||||
<input
|
||||
ref={fileInputRef}
|
||||
type="file"
|
||||
accept="audio/mpeg,audio/mp4,audio/aac"
|
||||
accept=".mp3,.m4a,.aac,audio/mpeg,audio/mp4,audio/aac"
|
||||
className="hidden"
|
||||
onChange={handleFileChange}
|
||||
/>
|
||||
|
||||
@@ -0,0 +1,26 @@
|
||||
/**
|
||||
* Client-safe constants for Story music-track uploads. Extracted out of
|
||||
* `supabase-storage.ts` (which imports `"server-only"` and therefore can't
|
||||
* be imported from the admin music page, a `"use client"` component) so the
|
||||
* upload form and the server-side storage/tRPC code share one source of
|
||||
* truth instead of two independently maintained copies drifting apart —
|
||||
* found during code review of the upload-validation fix (see
|
||||
* README.md's Known Issues).
|
||||
*/
|
||||
export const MUSIC_ALLOWED_CONTENT_TYPES = ["audio/mpeg", "audio/mp4", "audio/aac"] as const;
|
||||
export type MusicContentType = (typeof MUSIC_ALLOWED_CONTENT_TYPES)[number];
|
||||
|
||||
export const MAX_MUSIC_FILE_SIZE = 5 * 1024 * 1024; // 5 MB — short clips only
|
||||
|
||||
/**
|
||||
* Extension -> canonical content-type. A `Map`, not a plain object literal —
|
||||
* a plain-object lookup (`{ mp3: ... }[ext]`) resolves inherited
|
||||
* `Object.prototype` members for an extension like "constructor", silently
|
||||
* returning a function instead of `undefined` and bypassing the "unknown
|
||||
* extension" rejection entirely.
|
||||
*/
|
||||
export const MUSIC_EXTENSION_CONTENT_TYPES: ReadonlyMap<string, MusicContentType> = new Map([
|
||||
["mp3", "audio/mpeg"],
|
||||
["m4a", "audio/mp4"],
|
||||
["aac", "audio/aac"],
|
||||
]);
|
||||
@@ -1,5 +1,8 @@
|
||||
import "server-only";
|
||||
import { createClient, type SupabaseClient } from "@supabase/supabase-js";
|
||||
import { MUSIC_ALLOWED_CONTENT_TYPES, MAX_MUSIC_FILE_SIZE } from "./music-content-types";
|
||||
|
||||
export { MUSIC_ALLOWED_CONTENT_TYPES, MAX_MUSIC_FILE_SIZE };
|
||||
|
||||
// Lazy singleton — not initialized at module eval so Next.js build does not
|
||||
// throw when env vars are absent during the page-data collection phase.
|
||||
@@ -22,10 +25,14 @@ export const ALLOWED_CONTENT_TYPES = ["image/jpeg", "image/png", "image/webp"] a
|
||||
|
||||
// Music-track catalog (admin-curated, see admin.ts's music-track procedures)
|
||||
// — a separate bucket since it holds audio, not images. Must be created once
|
||||
// in the Supabase dashboard; this code assumes it already exists.
|
||||
// in the Supabase dashboard (name must match exactly — a "music-track" vs.
|
||||
// "music-tracks" typo here once made every upload fail with a bucket-not-found
|
||||
// error that surfaced as a generic upload failure, see README.md's Known
|
||||
// Issues); this code assumes it already exists.
|
||||
export const MUSIC_STORAGE_BUCKET = "music-tracks";
|
||||
export const MAX_MUSIC_FILE_SIZE = 5 * 1024 * 1024; // 5 MB — short clips only
|
||||
export const MUSIC_ALLOWED_CONTENT_TYPES = ["audio/mpeg", "audio/mp4", "audio/aac"] as const;
|
||||
// MAX_MUSIC_FILE_SIZE / MUSIC_ALLOWED_CONTENT_TYPES live in ./music-content-types
|
||||
// (a plain, non-"server-only" module) and are re-exported above so existing
|
||||
// imports of this file keep working — see that file's doc comment for why.
|
||||
|
||||
/**
|
||||
* Creates a signed upload URL for direct client-to-Supabase uploads.
|
||||
|
||||
Reference in new issue
Block a user