From 1d10e7a089624c7cf62ce22e9c43d7c662906c8c Mon Sep 17 00:00:00 2001 From: Fred <323546+fguillot@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:11:20 -0700 Subject: [PATCH] fix(config): accept spaces and trailing commas in list option values MEDIA_PROXY_RESOURCE_TYPES and TRUSTED_REVERSE_PROXY_NETWORKS were validated by splitting the raw value without trimming, while the parser trims items and skips empty ones. Values such as "image, video" or "192.168.0.0/16, 10.0.0.0/8", and lists with a trailing comma, failed startup even though the parser accepts them. Validate the parsed list instead, and reject values that contain no items at all so that a comma-only value cannot silently clear the default media proxy resource types. --- internal/config/options.go | 13 +++++++-- internal/config/options_parsing_test.go | 37 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/internal/config/options.go b/internal/config/options.go index 61054dc1..7b2ed692 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -4,6 +4,7 @@ package config // import "miniflux.app/v2/internal/config" import ( + "errors" "maps" "net" "net/url" @@ -383,7 +384,11 @@ func NewConfigOptions() *configOptions { rawValue: "image", valueType: stringListType, validator: func(rawValue string) error { - return validateListChoices(strings.Split(rawValue, ","), []string{"image", "video", "audio"}) + resourceTypes := parseStringListValue(rawValue, nil) + if len(resourceTypes) == 0 { + return errors.New("at least one resource type is required") + } + return validateListChoices(resourceTypes, []string{"image", "video", "audio"}) }, }, "METRICS_ALLOWED_NETWORKS": { @@ -569,7 +574,11 @@ func NewConfigOptions() *configOptions { rawValue: "", valueType: stringListType, validator: func(rawValue string) error { - for ip := range strings.SplitSeq(rawValue, ",") { + networks := parseStringListValue(rawValue, nil) + if len(networks) == 0 { + return errors.New("at least one CIDR notation network is required") + } + for _, ip := range networks { if _, _, err := net.ParseCIDR(ip); err != nil { return err } diff --git a/internal/config/options_parsing_test.go b/internal/config/options_parsing_test.go index 42abd8b2..cd5a8468 100644 --- a/internal/config/options_parsing_test.go +++ b/internal/config/options_parsing_test.go @@ -1508,9 +1508,26 @@ func TestMediaProxyResourceTypesOptionParsing(t *testing.T) { t.Fatalf("Expected MEDIA_PROXY_RESOURCE_TYPES to contain image and video") } + if err := configParser.parseLines([]string{"MEDIA_PROXY_RESOURCE_TYPES=image, video"}); err != nil { + t.Fatalf("Unexpected error for value with spaces: %v", err) + } + + resourceTypes = configParser.options.MediaProxyResourceTypes() + if len(resourceTypes) != 2 || resourceTypes[0] != "image" || resourceTypes[1] != "video" { + t.Fatalf("Expected MEDIA_PROXY_RESOURCE_TYPES to contain image and video") + } + + if err := configParser.parseLines([]string{"MEDIA_PROXY_RESOURCE_TYPES=image,video,"}); err != nil { + t.Fatalf("Unexpected error for value with trailing comma: %v", err) + } + if err := configParser.parseLines([]string{"MEDIA_PROXY_RESOURCE_TYPES=image,invalid,video"}); err == nil { t.Fatal("Expected error due to invalid resource type") } + + if err := configParser.parseLines([]string{"MEDIA_PROXY_RESOURCE_TYPES=,"}); err == nil { + t.Fatal("Expected error for value without any resource type") + } } func TestMetricsAllowedNetworksOptionParsing(t *testing.T) { @@ -1652,10 +1669,30 @@ func TestTrustedReverseProxyNetworksOptionParsing(t *testing.T) { t.Errorf("Expected 192.168.1.0/24 in allowed networks") } + // Test value with spaces and trailing comma + if err := configParser.parseLines([]string{"TRUSTED_REVERSE_PROXY_NETWORKS=192.168.0.0/16, 10.0.0.0/8,"}); err != nil { + t.Fatalf("Unexpected error for value with spaces: %v", err) + } + + allowedNetworks = configParser.options.TrustedReverseProxyNetworks() + if len(allowedNetworks) != 2 { + t.Fatalf("Expected 2 allowed networks, got %d", len(allowedNetworks)) + } + if !slices.Contains(allowedNetworks, "192.168.0.0/16") { + t.Errorf("Expected 192.168.0.0/16 in allowed networks") + } + if !slices.Contains(allowedNetworks, "10.0.0.0/8") { + t.Errorf("Expected 10.0.0.0/8 in allowed networks") + } + // Test invalid value if err := configParser.parseLines([]string{"TRUSTED_REVERSE_PROXY_NETWORKS=127.0.0.1"}); err == nil { t.Fatal("Expected error when parsing invalid CIDR notation IP 127.0.0.1, got nil") } + + if err := configParser.parseLines([]string{"TRUSTED_REVERSE_PROXY_NETWORKS=,"}); err == nil { + t.Fatal("Expected error for value without any network") + } } func TestYouTubeEmbedDomainOptionParsing(t *testing.T) {