diff --git a/weed/s3api/s3vectors/handlers.go b/weed/s3api/s3vectors/handlers.go index 2f02823b19..c887b9843e 100644 --- a/weed/s3api/s3vectors/handlers.go +++ b/weed/s3api/s3vectors/handlers.go @@ -951,6 +951,22 @@ func translateFilter(filter map[string]interface{}) (string, error) { return filterExpr(filter) } +// nonFilterableField returns the first blocked metadata key a filter +// references; the bool reports whether any matched. +func nonFilterableField(cfg *MetadataConfiguration, filter map[string]interface{}) (string, bool) { + if cfg == nil { + return "", false + } + fields := map[string]bool{} + filterFields(filter, fields) + for _, k := range cfg.NonFilterableMetadataKeys { + if fields[k] { + return k, true + } + } + return "", false +} + // filterFields collects the field names a filter expression references — // $-operators recurse; every other key is a field. func filterFields(filter map[string]interface{}, out map[string]bool) { @@ -1210,16 +1226,10 @@ func (s *Server) handleQueryVectors(w http.ResponseWriter, r *http.Request) { return } idx := indexFromRecord(s, arns.bucket, arns.index, record) - if cfg := idx.MetadataConfiguration; cfg != nil && len(cfg.NonFilterableMetadataKeys) > 0 { - fields := map[string]bool{} - filterFields(req.Filter, fields) - for _, k := range cfg.NonFilterableMetadataKeys { - if fields[k] { - writeError(w, http.StatusBadRequest, "ValidationException", - fmt.Sprintf("metadata key %q is not filterable", k)) - return - } - } + if key, blocked := nonFilterableField(idx.MetadataConfiguration, req.Filter); blocked { + writeError(w, http.StatusBadRequest, "ValidationException", + fmt.Sprintf("metadata key %q is not filterable", key)) + return } metric := idx.DistanceMetric filter, err := translateFilter(req.Filter) diff --git a/weed/s3api/s3vectors/handlers_test.go b/weed/s3api/s3vectors/handlers_test.go index d20e48a02a..f9b2433805 100644 --- a/weed/s3api/s3vectors/handlers_test.go +++ b/weed/s3api/s3vectors/handlers_test.go @@ -260,3 +260,87 @@ func TestTranslateFilter(t *testing.T) { t.Fatalf("empty filter: %q %v", got, err) } } + +// The catalog stores nonFilterableMetadataKeys as a JSON string; an older +// record may still carry the array form. Both must round-trip into the index. +func TestIndexFromRecordNonFilterableKeys(t *testing.T) { + s := &Server{} + record := func(keys interface{}) map[string]interface{} { + return map[string]interface{}{ + "metadata": map[string]interface{}{ + "fullMetadata": map[string]interface{}{ + "dimension": "128", + "distanceMetric": "cosine", + "nonFilterableMetadataKeys": keys, + }, + }, + } + } + + idx := indexFromRecord(s, "b", "i", record(`["sku","internal"]`)) + if idx.MetadataConfiguration == nil || + len(idx.MetadataConfiguration.NonFilterableMetadataKeys) != 2 || + idx.MetadataConfiguration.NonFilterableMetadataKeys[0] != "sku" || + idx.MetadataConfiguration.NonFilterableMetadataKeys[1] != "internal" { + t.Fatalf("JSON string form: %+v", idx.MetadataConfiguration) + } + + idx = indexFromRecord(s, "b", "i", record([]interface{}{"sku"})) + if idx.MetadataConfiguration == nil || + len(idx.MetadataConfiguration.NonFilterableMetadataKeys) != 1 || + idx.MetadataConfiguration.NonFilterableMetadataKeys[0] != "sku" { + t.Fatalf("array form: %+v", idx.MetadataConfiguration) + } + + for _, absent := range []interface{}{nil, "not-json", `{"a":1}`} { + idx = indexFromRecord(s, "b", "i", record(absent)) + if idx.MetadataConfiguration != nil { + t.Fatalf("keys %v should leave configuration unset, got %+v", absent, idx.MetadataConfiguration) + } + } +} + +// QueryVectors rejects filters touching non-filterable keys; the extraction +// must see through nested boolean operators. +func TestFilterFieldsNestedOperators(t *testing.T) { + filter := map[string]interface{}{ + "$and": []interface{}{ + map[string]interface{}{"color": "red"}, + map[string]interface{}{"$or": []interface{}{ + map[string]interface{}{"sku": map[string]interface{}{"$eq": "x"}}, + map[string]interface{}{"$and": []interface{}{ + map[string]interface{}{"size": map[string]interface{}{"$gt": 3}}, + }}, + }}, + }, + } + fields := map[string]bool{} + filterFields(filter, fields) + for _, want := range []string{"color", "sku", "size"} { + if !fields[want] { + t.Fatalf("missing field %q in %v", want, fields) + } + } + if fields["$and"] || fields["$or"] || fields["$eq"] { + t.Fatalf("operators leaked into fields: %v", fields) + } + + // The rejection gate the handler applies to the extracted fields. + blocked := &MetadataConfiguration{NonFilterableMetadataKeys: []string{"sku"}} + if got, hit := nonFilterableField(blocked, filter); !hit || got != "sku" { + t.Fatalf("blocked key inside nested filter not detected: %q %v", got, hit) + } + allowed := &MetadataConfiguration{NonFilterableMetadataKeys: []string{"secret"}} + if got, hit := nonFilterableField(allowed, filter); hit { + t.Fatalf("unrelated non-filterable key should pass, got %q", got) + } + if got, hit := nonFilterableField(nil, filter); hit { + t.Fatalf("no configuration should pass, got %q", got) + } + emptyBlocked := &MetadataConfiguration{NonFilterableMetadataKeys: []string{""}} + if _, hit := nonFilterableField(emptyBlocked, map[string]interface{}{ + "": map[string]interface{}{"$eq": "x"}, + }); !hit { + t.Fatal("an empty blocked key should still reject") + } +}