fix(cmd): reject malformed subscribe filters

cmd/subscribe.go

@@ -59,35 +59,43 @@ func runSubscribe(cmd *cobra.Command) (err error) {
 	}
 
 	// add subscription	to function
-	f.Deploy.Subscriptions = updateOrAddSubscription(f.Deploy.Subscriptions, cfg)
+	subscriptions, err := updateOrAddSubscription(f.Deploy.Subscriptions, cfg)
+	if err != nil {
+		return err
+	}
+	f.Deploy.Subscriptions = subscriptions
 
 	// pump it
 	return f.Write()
 }
 
-func extractFilterMap(filters []string) map[string]string {
+func extractFilterMap(filters []string) (map[string]string, error) {
 	subscriptionFilters := make(map[string]string)
 	for _, filter := range filters {
-		kv := strings.Split(filter, "=")
+		kv := strings.SplitN(filter, "=", 2)
 		if len(kv) != 2 {
-			fmt.Println("Invalid pair:", filter)
-			continue
+			return nil, fmt.Errorf("invalid filter %q: must be in the form key=value", filter)
+		}
+		key, value := kv[0], kv[1]
+		if key == "" {
+			return nil, fmt.Errorf("invalid filter %q: key must not be empty", filter)
 		}
-		key := kv[0]
-		value := kv[1]
 		subscriptionFilters[key] = value
 	}
-	return subscriptionFilters
+	return subscriptionFilters, nil
 }
 
 type subscibeConfig struct {
 	Filter []string
 	Source string
 }
 
-func updateOrAddSubscription(subscriptions []fn.KnativeSubscription, cfg subscibeConfig) []fn.KnativeSubscription {
+func updateOrAddSubscription(subscriptions []fn.KnativeSubscription, cfg subscibeConfig) ([]fn.KnativeSubscription, error) {
 	found := false
-	newFilters := extractFilterMap(cfg.Filter)
+	newFilters, err := extractFilterMap(cfg.Filter)
+	if err != nil {
+		return nil, err
+	}
 
 	// Iterate over subscriptions to find if one with the same source already exists
 	for i, subscription := range subscriptions {
@@ -114,7 +122,7 @@ func updateOrAddSubscription(subscriptions []fn.KnativeSubscription, cfg subscib
 			Filters: newFilters,
 		})
 	}
-	return subscriptions
+	return subscriptions, nil
 }
 
 func newSubscribeConfig(cmd *cobra.Command) (c subscibeConfig) {

cmd/subscribe_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"reflect"
 	"testing"
 
 	fn "knative.dev/func/pkg/functions"
@@ -286,3 +287,134 @@ func TestSubscribeWithDuplicated(t *testing.T) {
 	}
 
 }
+
+// TestSubscribeRejectsMalformedFilter ensures that a --filter value which is
+// not in key=value form fails the command rather than being silently dropped,
+// and that nothing is written to func.yaml.
+func TestSubscribeRejectsMalformedFilter(t *testing.T) {
+	root := FromTempDirectory(t)
+
+	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "badfilter"})
+
+	if err := cmd.Execute(); err == nil {
+		t.Fatal("Expected an error for a malformed filter, but got nil")
+	}
+
+	// The function on disk must be left untouched.
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(f.Deploy.Subscriptions) != 0 {
+		t.Fatalf("Expected no subscriptions to be written, but got '%v'", f.Deploy.Subscriptions)
+	}
+}
+
+// TestSubscribeAllowsFilterValueContainingEquals ensures a filter value which
+// itself contains "=" is preserved in full rather than discarded.
+func TestSubscribeAllowsFilterValueContainingEquals(t *testing.T) {
+	root := FromTempDirectory(t)
+
+	_, err := fn.New().Init(fn.Function{Runtime: "go", Root: root})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := NewSubscribeCmd()
+	cmd.SetArgs([]string{"--source", "my-broker", "--filter", "foo=bar=baz"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := fn.NewFunction(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if f.Deploy.Subscriptions == nil {
+		t.Fatal("Expected subscription to be present ")
+	}
+	if f.Deploy.Subscriptions[0].Filters["foo"] != "bar=baz" {
+		t.Fatalf("Expected subscription filter for 'foo' to be 'bar=baz', but got '%v'", f.Deploy.Subscriptions[0].Filters["foo"])
+	}
+}
+
+// TestExtractFilterMap covers filter parsing directly, including values which
+// themselves contain "=" and the malformed forms which must be rejected.
+func TestExtractFilterMap(t *testing.T) {
+	tests := []struct {
+		name    string
+		filters []string
+		want    map[string]string
+		wantErr bool
+	}{
+		{
+			name:    "single valid filter",
+			filters: []string{"type=com.example"},
+			want:    map[string]string{"type": "com.example"},
+		},
+		{
+			name:    "multiple valid filters",
+			filters: []string{"type=com.example", "extension=my-value"},
+			want:    map[string]string{"type": "com.example", "extension": "my-value"},
+		},
+		{
+			name:    "value containing equals is preserved",
+			filters: []string{"foo=bar=baz"},
+			want:    map[string]string{"foo": "bar=baz"},
+		},
+		{
+			name:    "empty value is allowed",
+			filters: []string{"key="},
+			want:    map[string]string{"key": ""},
+		},
+		{
+			name:    "no filters yields empty map",
+			filters: []string{},
+			want:    map[string]string{},
+		},
+		{
+			name:    "missing equals is rejected",
+			filters: []string{"badfilter"},
+			wantErr: true,
+		},
+		{
+			name:    "empty key is rejected",
+			filters: []string{"=value"},
+			wantErr: true,
+		},
+		{
+			name:    "one malformed filter rejects the whole set",
+			filters: []string{"type=com.example", "badfilter"},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := extractFilterMap(tt.filters)
+
+			if tt.wantErr {
+				if err == nil {
+					t.Fatalf("Expected an error, but got nil (result '%v')", got)
+				}
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Expected '%v', but got '%v'", tt.want, got)
+			}
+		})
+	}
+}