From 201dd2fc0f709d72d5600ad1bfb54e7e628e0274 Mon Sep 17 00:00:00 2001 From: "Tomi P. Hakala" Date: Tue, 21 Jan 2025 19:19:22 +0200 Subject: [PATCH 1/6] refactor: enhance audio clip saving logic in actions.go and processor.go - Updated SaveAudioAction to construct the full output path for audio clips by combining the export path with the clip name, ensuring proper file organization. - Added directory creation logic to ensure the output path exists before saving, improving error handling and robustness. - Simplified clip name generation in processor.go by removing unnecessary base path handling, streamlining the process of constructing clip names for audio files. --- internal/analysis/processor/actions.go | 13 ++++++++++--- internal/analysis/processor/processor.go | 7 ++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/internal/analysis/processor/actions.go b/internal/analysis/processor/actions.go index 7115cd3e..b6da9310 100644 --- a/internal/analysis/processor/actions.go +++ b/internal/analysis/processor/actions.go @@ -7,6 +7,8 @@ import ( "encoding/json" "fmt" "log" + "os" + "path/filepath" "strings" "time" @@ -134,7 +136,14 @@ func (a DatabaseAction) Execute(data interface{}) error { // Execute saves the audio clip to a file func (a SaveAudioAction) Execute(data interface{}) error { - outputPath := a.ClipName + // Get the full path by joining the export path with the relative clip name + outputPath := filepath.Join(a.Settings.Realtime.Audio.Export.Path, a.ClipName) + + // Ensure the directory exists + if err := os.MkdirAll(filepath.Dir(outputPath), 0755); err != nil { + log.Printf("error creating directory for audio clip: %s\n", err) + return err + } if a.Settings.Realtime.Audio.Export.Type == "wav" { if err := myaudio.SavePCMDataToWAV(outputPath, a.pcmData); err != nil { @@ -148,8 +157,6 @@ func (a SaveAudioAction) Execute(data interface{}) error { } } - log.Printf("Saved audio clip to %s\n", outputPath) - if a.Settings.Debug { log.Printf("Saved audio clip to %s\n", outputPath) } diff --git a/internal/analysis/processor/processor.go b/internal/analysis/processor/processor.go index 78c66915..aa277b84 100644 --- a/internal/analysis/processor/processor.go +++ b/internal/analysis/processor/processor.go @@ -308,9 +308,6 @@ func (p *Processor) getBaseConfidenceThreshold(speciesLowercase string) float32 // generateClipName generates a clip name for the given scientific name and confidence. func (p *Processor) generateClipName(scientificName string, confidence float32) string { - // Get the base path from the configuration - basePath := p.Settings.Realtime.Audio.Export.Path - // Replace whitespaces with underscores and convert to lowercase formattedName := strings.ToLower(strings.ReplaceAll(scientificName, " ", "_")) @@ -332,8 +329,8 @@ func (p *Processor) generateClipName(scientificName string, confidence float32) fileType := myaudio.GetFileExtension(p.Settings.Realtime.Audio.Export.Type) // Construct the clip name with the new pattern, including year and month subdirectories - // Use filepath.ToSlash to convert the path to a forward slash on Windows to avoid issues with URL encoding - clipName := filepath.ToSlash(filepath.Join(basePath, year, month, fmt.Sprintf("%s_%s_%s.%s", formattedName, formattedConfidence, timestamp, fileType))) + // Use filepath.ToSlash to convert the path to a forward slash for web URLs + clipName := filepath.ToSlash(filepath.Join(year, month, fmt.Sprintf("%s_%s_%s.%s", formattedName, formattedConfidence, timestamp, fileType))) return clipName } From 5f94dd8a98148de5106c921e301ae572fc2285fd Mon Sep 17 00:00:00 2001 From: "Tomi P. Hakala" Date: Tue, 21 Jan 2025 19:19:36 +0200 Subject: [PATCH 2/6] refactor: enhance cache control and media handling in middleware and handlers - Updated CacheControlMiddleware to use server context for improved logging and cache header management based on request paths. - Added support for audio file handling in ServeAudioClip, including sanitization and MIME type determination. - Refactored sanitizeClipName to ensure relative path handling and added debug logging for better traceability. - Introduced new utility functions for path management to streamline audio file serving and enhance code clarity. --- internal/httpcontroller/handlers/media.go | 146 ++++++++++++++++++++-- internal/httpcontroller/middleware.go | 28 ++++- internal/httpcontroller/routes.go | 8 +- internal/httpcontroller/server.go | 11 ++ 4 files changed, 179 insertions(+), 14 deletions(-) diff --git a/internal/httpcontroller/handlers/media.go b/internal/httpcontroller/handlers/media.go index a1f44811..1da6b687 100644 --- a/internal/httpcontroller/handlers/media.go +++ b/internal/httpcontroller/handlers/media.go @@ -7,6 +7,7 @@ import ( "html" "html/template" "log" + "net/http" "net/url" "os" "os/exec" @@ -33,8 +34,8 @@ var ( ErrPathTraversal = errors.New("path traversal attempt detected") ) -// sanitizeClipName performs sanity checks on the clip name -func sanitizeClipName(clipName string) (string, error) { +// sanitizeClipName performs sanity checks on the clip name and ensures it's a relative path +func (h *Handlers) sanitizeClipName(clipName string) (string, error) { // Check if the clip name is empty if clipName == "" { return "", ErrEmptyClipName @@ -45,6 +46,7 @@ func sanitizeClipName(clipName string) (string, error) { if err != nil { return "", fmt.Errorf("error decoding clip name: %w", err) } + h.Debug("sanitizeClipName: Decoded clip name: %s", decodedClipName) // Check the length of the decoded clip name if len(decodedClipName) > MaxClipNameLength { @@ -53,18 +55,66 @@ func sanitizeClipName(clipName string) (string, error) { // Check for allowed characters if !regexp.MustCompile(AllowedCharacters).MatchString(decodedClipName) { + h.Debug("sanitizeClipName: Invalid characters in clip name: %s", decodedClipName) return "", ErrInvalidCharacters } - // Check for potential path traversal attempts + // Clean the path and ensure it's relative cleanPath := filepath.Clean(decodedClipName) + h.Debug("sanitizeClipName: Cleaned path: %s", cleanPath) + if strings.Contains(cleanPath, "..") { + h.Debug("sanitizeClipName: Path traversal attempt detected: %s", cleanPath) return "", ErrPathTraversal } + // Remove 'clips/' prefix if present + cleanPath = strings.TrimPrefix(cleanPath, "clips/") + h.Debug("sanitizeClipName: Path after removing clips prefix: %s", cleanPath) + + // If the path is absolute, make it relative to the export path + if filepath.IsAbs(cleanPath) { + h.Debug("sanitizeClipName: Found absolute path: %s", cleanPath) + exportPath := conf.Setting().Realtime.Audio.Export.Path + h.Debug("sanitizeClipName: Export path from settings: %s", exportPath) + + if strings.HasPrefix(cleanPath, exportPath) { + // Remove the export path prefix to make it relative + cleanPath = strings.TrimPrefix(cleanPath, exportPath) + cleanPath = strings.TrimPrefix(cleanPath, string(os.PathSeparator)) + h.Debug("sanitizeClipName: Converted to relative path: %s", cleanPath) + } else { + h.Debug("sanitizeClipName: Absolute path not under export directory: %s", cleanPath) + return "", fmt.Errorf("invalid path: absolute path not under export directory") + } + } + + // Convert to forward slashes for web URLs + cleanPath = filepath.ToSlash(cleanPath) + h.Debug("sanitizeClipName: Final path with forward slashes: %s", cleanPath) + return cleanPath, nil } +// getFullPath returns the full filesystem path for a relative clip path +func getFullPath(relativePath string) string { + exportPath := conf.Setting().Realtime.Audio.Export.Path + return filepath.Join(exportPath, relativePath) +} + +// getWebPath converts a filesystem path to a web-safe path +func getWebPath(path string) string { + // Convert absolute path to relative path if it starts with the export path + exportPath := conf.Setting().Realtime.Audio.Export.Path + if strings.HasPrefix(path, exportPath) { + path = strings.TrimPrefix(path, exportPath) + path = strings.TrimPrefix(path, string(os.PathSeparator)) + } + + // Convert path separators to forward slashes for web URLs + return filepath.ToSlash(path) +} + // Thumbnail returns the URL of a given bird's thumbnail image. // It takes the bird's scientific name as input and returns the image URL as a string. // If the image is not found or an error occurs, it returns an empty string. @@ -125,16 +175,19 @@ func (h *Handlers) ServeSpectrogram(c echo.Context) error { clipName := c.QueryParam("clip") // Sanitize the clip name - sanitizedClipName, err := sanitizeClipName(clipName) + sanitizedClipName, err := h.sanitizeClipName(clipName) if err != nil { - log.Printf("Error sanitizing clip name: %v", err) + h.Debug("Error sanitizing clip name: %v", err) return c.File("assets/images/spectrogram-placeholder.svg") } + // Get the full path to the audio file + fullPath := getFullPath(sanitizedClipName) + // Construct the path to the spectrogram image - spectrogramPath, err := h.getSpectrogramPath(sanitizedClipName, 400) // Assuming 400px width + spectrogramPath, err := h.getSpectrogramPath(fullPath, 400) // Assuming 400px width if err != nil { - log.Printf("Error getting spectrogram path: %v", err) + h.Debug("Error getting spectrogram path: %v", err) return c.File("assets/images/spectrogram-placeholder.svg") } @@ -389,3 +442,82 @@ func createSpectrogramWithFFmpeg(audioClipPath, spectrogramPath string, width in return nil } + +// ServeAudioClip serves an audio clip file +func (h *Handlers) ServeAudioClip(c echo.Context) error { + h.Debug("ServeAudioClip: Starting to handle request for path: %s", c.Request().URL.String()) + + // Extract clip name from the query parameters + clipName := c.QueryParam("clip") + h.Debug("ServeAudioClip: Raw clip name from query: %s", clipName) + + // Sanitize the clip name + sanitizedClipName, err := h.sanitizeClipName(clipName) + if err != nil { + h.Debug("ServeAudioClip: Error sanitizing clip name: %v", err) + c.Response().Header().Set(echo.HeaderContentType, "text/plain") + return c.String(http.StatusNotFound, "Audio file not found") + } + h.Debug("ServeAudioClip: Sanitized clip name: %s", sanitizedClipName) + + // Get the full path to the audio file + fullPath := getFullPath(sanitizedClipName) + h.Debug("ServeAudioClip: Full path: %s", fullPath) + + // Check if the file exists + if _, err := os.Stat(fullPath); err != nil { + if os.IsNotExist(err) { + h.Debug("ServeAudioClip: Audio file not found: %s", fullPath) + } else { + h.Debug("ServeAudioClip: Error checking audio file: %v", err) + } + c.Response().Header().Set(echo.HeaderContentType, "text/plain") + return c.String(http.StatusNotFound, "Audio file not found") + } + h.Debug("ServeAudioClip: File exists at path: %s", fullPath) + + // Get the filename for Content-Disposition + filename := filepath.Base(sanitizedClipName) + h.Debug("ServeAudioClip: Using filename for disposition: %s", filename) + + // Get MIME type + mimeType := getAudioMimeType(fullPath) + h.Debug("ServeAudioClip: MIME type for file: %s", mimeType) + + // Set response headers + c.Response().Header().Set(echo.HeaderContentType, mimeType) + c.Response().Header().Set("Content-Transfer-Encoding", "binary") + c.Response().Header().Set("Content-Description", "File Transfer") + c.Response().Header().Set(echo.HeaderContentDisposition, fmt.Sprintf(`attachment; filename="%s"`, filename)) + + h.Debug("ServeAudioClip: Set headers - Content-Type: %s, Content-Disposition: %s", + c.Response().Header().Get(echo.HeaderContentType), + c.Response().Header().Get(echo.HeaderContentDisposition)) + + // Serve the file + h.Debug("ServeAudioClip: Attempting to serve file: %s", fullPath) + return c.File(fullPath) +} + +// getAudioMimeType returns the MIME type for an audio file based on its extension +func getAudioMimeType(filename string) string { + ext := strings.ToLower(filepath.Ext(filename)) + switch ext { + case ".mp3": + return "audio/mpeg" + case ".ogg", ".opus": + return "audio/ogg" + case ".wav": + return "audio/wav" + case ".flac": + return "audio/flac" + case ".aac": + return "audio/aac" + case ".m4a": + return "audio/mp4" + case ".alac": + return "audio/x-alac" + default: + return "application/octet-stream" + } +} diff --git a/internal/httpcontroller/middleware.go b/internal/httpcontroller/middleware.go index 82308ed5..4eca77bb 100644 --- a/internal/httpcontroller/middleware.go +++ b/internal/httpcontroller/middleware.go @@ -20,31 +20,51 @@ func (s *Server) configureMiddleware() { MinLength: 2048, })) // Apply the Cache Control Middleware - s.Echo.Use(CacheControlMiddleware()) + s.Echo.Use(s.CacheControlMiddleware()) s.Echo.Use(VaryHeaderMiddleware()) } -func CacheControlMiddleware() echo.MiddlewareFunc { +// CacheControlMiddleware sets appropriate cache control headers based on the request path +func (s *Server) CacheControlMiddleware() echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { path := c.Request().URL.Path + s.Debug("CacheControlMiddleware: Processing request for path: %s", path) switch { case strings.HasSuffix(path, ".css"), strings.HasSuffix(path, ".js"), strings.HasSuffix(path, ".html"): // CSS and JS files - shorter cache with validation c.Response().Header().Set("Cache-Control", "public, max-age=3600, must-revalidate") c.Response().Header().Set("ETag", generateETag(path)) + s.Debug("CacheControlMiddleware: Set cache headers for static file: %s", path) case strings.HasSuffix(path, ".png"), strings.HasSuffix(path, ".jpg"), strings.HasSuffix(path, ".ico"), strings.HasSuffix(path, ".svg"): // Images can be cached longer c.Response().Header().Set("Cache-Control", "public, max-age=604800, immutable") - case strings.HasPrefix(path, "/clips/"): + s.Debug("CacheControlMiddleware: Set cache headers for image: %s", path) + case strings.HasPrefix(path, "/media/audio"): + // Audio files - set proper headers for downloads + c.Response().Header().Set("Cache-Control", "private, no-cache") + c.Response().Header().Set("X-Content-Type-Options", "nosniff") + s.Debug("CacheControlMiddleware: Set headers for audio file: %s", path) + s.Debug("CacheControlMiddleware: Headers after setting - Cache-Control: %s, X-Content-Type-Options: %s", + c.Response().Header().Get("Cache-Control"), + c.Response().Header().Get("X-Content-Type-Options")) + case strings.HasPrefix(path, "/media/spectrogram"): + // Spectrograms can be cached c.Response().Header().Set("Cache-Control", "public, max-age=2592000, immutable") + s.Debug("CacheControlMiddleware: Set cache headers for spectrogram: %s", path) default: // Dynamic content c.Response().Header().Set("Cache-Control", "private, no-cache, must-revalidate") + s.Debug("CacheControlMiddleware: Set default cache headers for: %s", path) } - return next(c) + + err := next(c) + if err != nil { + s.Debug("CacheControlMiddleware: Error processing request: %v", err) + } + return err } } } diff --git a/internal/httpcontroller/routes.go b/internal/httpcontroller/routes.go index 20818afb..a49a386f 100644 --- a/internal/httpcontroller/routes.go +++ b/internal/httpcontroller/routes.go @@ -7,6 +7,7 @@ import ( "html/template" "io/fs" "net/http" + "strings" "github.com/labstack/echo/v4" "github.com/tphakala/birdnet-go/internal/conf" @@ -91,14 +92,16 @@ func (s *Server) initRoutes() { "/top-birds": {Path: "/top-birds", TemplateName: "birdsTableHTML", Title: "Top Birds", Handler: h.WithErrorHandling(h.TopBirds)}, "/notes": {Path: "/notes", TemplateName: "notes", Title: "All Notes", Handler: h.WithErrorHandling(h.GetAllNotes)}, "/media/spectrogram": {Path: "/media/spectrogram", TemplateName: "", Title: "", Handler: h.WithErrorHandling(h.ServeSpectrogram)}, + "/media/audio": {Path: "/media/audio", TemplateName: "", Title: "", Handler: h.WithErrorHandling(h.ServeAudioClip)}, "/login": {Path: "/login", TemplateName: "login", Title: "Login", Handler: h.WithErrorHandling(s.handleLoginPage)}, } // Set up partial routes for _, route := range s.partialRoutes { s.Echo.GET(route.Path, func(c echo.Context) error { - // If the request is a hx-request or spectrogram, call the partial route handler - if c.Request().Header.Get("HX-Request") != "" || c.Request().URL.Path == "/media/spectrogram" { + // If the request is a hx-request or media request, call the partial route handler + if c.Request().Header.Get("HX-Request") != "" || + strings.HasPrefix(c.Request().URL.Path, "/media/") { return route.Handler(c) } else { // Call the full page route handler @@ -187,5 +190,4 @@ func (s *Server) setupStaticFileServing() { s.Echo.Logger.Fatal(err) } s.Echo.StaticFS("/assets", echo.MustSubFS(assetsFS, "")) - s.Echo.Static("/clips", "clips") } diff --git a/internal/httpcontroller/server.go b/internal/httpcontroller/server.go index 646d4859..3d32e55c 100644 --- a/internal/httpcontroller/server.go +++ b/internal/httpcontroller/server.go @@ -218,3 +218,14 @@ func (s *Server) initLogger() { }, })) } + +// Debug logs debug messages if debug mode is enabled +func (s *Server) Debug(format string, v ...interface{}) { + if s.Settings.WebServer.Debug { + if len(v) == 0 { + log.Print(format) + } else { + log.Printf(format, v...) + } + } +} From fcf09ef8e2847345983e84965a9c5ecf6b8495fc Mon Sep 17 00:00:00 2001 From: "Tomi P. Hakala" Date: Tue, 21 Jan 2025 19:19:45 +0200 Subject: [PATCH 3/6] refactor: update audio clip references in HTML templates for consistency - Changed audio and spectrogram image source URLs in detectionDetails.html, listDetections.html, and recentDetections.html to use the correct media paths. - Replaced instances of `urlsafe .ClipName` with `.ClipName` for improved clarity and consistency across the templates. - Ensured that audio download links point to the correct media endpoint, enhancing the functionality of the audio player components. --- views/fragments/detectionDetails.html | 6 +++--- views/fragments/listDetections.html | 6 +++--- views/fragments/recentDetections.html | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/views/fragments/detectionDetails.html b/views/fragments/detectionDetails.html index ab0e2919..4f03de35 100644 --- a/views/fragments/detectionDetails.html +++ b/views/fragments/detectionDetails.html @@ -20,7 +20,7 @@

- Spectrogram @@ -30,7 +30,7 @@

- +
0:00 - diff --git a/views/fragments/listDetections.html b/views/fragments/listDetections.html index e881c3a2..ceb1b1ed 100644 --- a/views/fragments/listDetections.html +++ b/views/fragments/listDetections.html @@ -140,7 +140,7 @@
- Spectrogram Image @@ -151,7 +151,7 @@