From 827b370ac1bbcabecfae6e58437ff3eec18ad333 Mon Sep 17 00:00:00 2001 From: J3vb Date: Thu, 2 Apr 2026 13:45:41 +0200 Subject: [PATCH] fix: audio cleanup srcObject, diagnostics rate limit, orphan cleanup race (BUG-107, BUG-121, BUG-132) BUG-107: cleanupAllAudioElements now calls pause() and sets srcObject = null before removing elements from DOM, ensuring streams are fully released during reconnection cleanup. BUG-121: Diagnostics endpoint now has 5 req/min rate limit as documented, preventing enumeration of internal topology. BUG-132: DeleteOrphanedAttachments uses DELETE ... RETURNING stored_as (atomic) instead of separate SELECT then DELETE, eliminating the race where a file could be deleted after its attachment was linked. --- Client/tauri-client/src/lib/audioElements.ts | 13 ++++++++-- Server/api/router.go | 7 ++++-- Server/db/attachment_queries.go | 25 ++++++++------------ 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/Client/tauri-client/src/lib/audioElements.ts b/Client/tauri-client/src/lib/audioElements.ts index 0374a377..7e2335bf 100644 --- a/Client/tauri-client/src/lib/audioElements.ts +++ b/Client/tauri-client/src/lib/audioElements.ts @@ -225,10 +225,19 @@ export class AudioElements { /** Remove all remote audio elements from the DOM and clear tracking maps. * Preserves screenshare mute state so reconnecting tracks inherit user intent. */ cleanupAllAudioElements(): void { - for (const el of this.remoteMicAudioElements.values()) el.remove(); + // BUG-107: Fully release audio elements — pause, clear srcObject, then remove. + for (const el of this.remoteMicAudioElements.values()) { + el.pause(); + el.srcObject = null; + el.remove(); + } this.remoteMicAudioElements.clear(); for (const audioEls of this.screenshareAudioElements.values()) { - for (const el of audioEls) el.remove(); + for (const el of audioEls) { + el.pause(); + el.srcObject = null; + el.remove(); + } } this.screenshareAudioElements.clear(); } diff --git a/Server/api/router.go b/Server/api/router.go index 04ef8dbf..9692228e 100644 --- a/Server/api/router.go +++ b/Server/api/router.go @@ -153,8 +153,11 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri MountDMRoutes(r, database, hub) // Connectivity diagnostics — any authenticated user can check. - r.With(AuthMiddleware(database)).Get("/api/v1/diagnostics/connectivity", - handleDiagnosticsConnectivity(cfg, ver, hub)) + // BUG-121: Rate limit 5 req/min as documented. + r.With(AuthMiddleware(database), + RateLimitMiddleware(limiter, 5, time.Minute, cfg.Server.TrustedProxies)). + Get("/api/v1/diagnostics/connectivity", + handleDiagnosticsConnectivity(cfg, ver, hub)) go hub.Run() r.Get("/api/v1/ws", ws.ServeWS(hub, database, cfg.Server.AllowedOrigins)) diff --git a/Server/db/attachment_queries.go b/Server/db/attachment_queries.go index 352bb485..d8da2d1e 100644 --- a/Server/db/attachment_queries.go +++ b/Server/db/attachment_queries.go @@ -159,16 +159,21 @@ func (d *DB) GetAttachmentsByMessageIDs(msgIDs []int64) (map[int64][]AttachmentI return result, nil } -// DeleteOrphanedAttachments removes attachment records where message_id IS NULL -// and uploaded_at is older than the given cutoff time string (ISO 8601). -// Returns the stored_as filenames of deleted records so the caller can remove files. +// DeleteOrphanedAttachments atomically removes attachment records where +// message_id IS NULL and uploaded_at is older than the given cutoff time +// string (ISO 8601). Returns the stored_as filenames of deleted records +// so the caller can remove files. +// +// BUG-132: Uses DELETE ... RETURNING to make select+delete atomic, +// preventing a race where an attachment linked between SELECT and DELETE +// would have its file deleted while the DB row survives. func (d *DB) DeleteOrphanedAttachments(cutoff string) ([]string, error) { rows, err := d.sqlDB.Query( - `SELECT stored_as FROM attachments WHERE message_id IS NULL AND uploaded_at < ?`, + `DELETE FROM attachments WHERE message_id IS NULL AND uploaded_at < ? RETURNING stored_as`, cutoff, ) if err != nil { - return nil, fmt.Errorf("DeleteOrphanedAttachments query: %w", err) + return nil, fmt.Errorf("DeleteOrphanedAttachments: %w", err) } defer rows.Close() //nolint:errcheck @@ -184,15 +189,5 @@ func (d *DB) DeleteOrphanedAttachments(cutoff string) ([]string, error) { return nil, fmt.Errorf("DeleteOrphanedAttachments rows: %w", rows.Err()) } - if len(files) > 0 { - _, err = d.sqlDB.Exec( - `DELETE FROM attachments WHERE message_id IS NULL AND uploaded_at < ?`, - cutoff, - ) - if err != nil { - return nil, fmt.Errorf("DeleteOrphanedAttachments delete: %w", err) - } - } - return files, nil }