From b2e18fde130ee87d8bce5bf091fece95ae54c5b0 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 27 Oct 2026 13:55:33 +0100 Subject: [PATCH] fix(search): fill a repeated root placeholder everywhere, reject a repeated or grouped scope --- services/search/pkg/search/scope.go | 26 ++++++++++--- services/search/pkg/search/scope_test.go | 9 +++++ services/search/pkg/search/template.go | 42 ++++++++++----------- services/search/pkg/search/template_test.go | 17 +++++++++ 4 files changed, 66 insertions(+), 28 deletions(-) diff --git a/services/search/pkg/search/scope.go b/services/search/pkg/search/scope.go index 36597ec658..643a02f1d6 100644 --- a/services/search/pkg/search/scope.go +++ b/services/search/pkg/search/scope.go @@ -8,18 +8,32 @@ import ( "github.com/opencloud-eu/opencloud/pkg/kql" ) -// ErrNestedScope is returned when the scope sits inside a group: it narrows -// the whole search to a folder and cannot take part in a boolean expression. -var ErrNestedScope = errors.New("scope: must be a top-level restriction") +// A scope narrows the whole search to one folder, so it cannot take part in a +// boolean expression and cannot be given twice. +var ( + ErrNestedScope = errors.New("scope: must be a top-level restriction") + ErrRepeatedScope = errors.New("scope: must be given once") +) // ExtractScope takes the scope restriction out of the query, together with // the operator that joined it, and returns the remaining nodes and the scope. func ExtractScope(nodes []ast.Node) ([]ast.Node, string, error) { + scopes := 0 for _, n := range nodes { - if group, ok := n.(*ast.GroupNode); ok && hasKey(group.Nodes, "scope") { - return nil, "", ErrNestedScope + switch node := n.(type) { + case *ast.GroupNode: + if strings.EqualFold(node.Key, "scope") || hasKey(node.Nodes, "scope") { + return nil, "", ErrNestedScope + } + case *ast.StringNode: + if strings.EqualFold(node.Key, "scope") { + scopes++ + } } } + if scopes > 1 { + return nil, "", ErrRepeatedScope + } for i, n := range nodes { node, ok := n.(*ast.StringNode) @@ -67,7 +81,7 @@ func hasKey(nodes []ast.Node, key string) bool { return true } case *ast.GroupNode: - if hasKey(node.Nodes, key) { + if strings.EqualFold(node.Key, key) || hasKey(node.Nodes, key) { return true } } diff --git a/services/search/pkg/search/scope_test.go b/services/search/pkg/search/scope_test.go index aab6451a9a..fb4eae7287 100644 --- a/services/search/pkg/search/scope_test.go +++ b/services/search/pkg/search/scope_test.go @@ -38,8 +38,17 @@ var _ = DescribeTable("ExtractScope rejects a nested scope", Entry("inside a group", `name:x AND (scope:1$2!3 OR tag:y)`), Entry("inside a nested group", `name:x AND (tag:y OR (scope:1$2!3))`), Entry("after a top-level scope", `scope:1$2!3 AND (scope:1$2!4)`), + Entry("as a group value", `name:x AND scope:(1$2!3)`), + Entry("as a group value inside a group", `name:x AND (tag:y OR scope:(1$2!3))`), ) +var _ = Describe("ExtractScope", func() { + It("rejects a second scope", func() { + _, _, err := search.ExtractScope(parse(`scope:1$2!3 AND name:x AND scope:1$2!4`).Nodes) + Expect(err).To(MatchError(search.ErrRepeatedScope)) + }) +}) + var _ = DescribeTable("ExpandFavorites", func(query, want string) { a := parse(query) diff --git a/services/search/pkg/search/template.go b/services/search/pkg/search/template.go index 64eda76fb7..ecc9be1d2d 100644 --- a/services/search/pkg/search/template.go +++ b/services/search/pkg/search/template.go @@ -17,12 +17,10 @@ import ( // nodes where the user's query and the space roots go. An empty template // expands to the bare query. type QueryTemplate struct { - nodes []ast.Node - query *ast.GroupNode - queries int - stray bool - personal *ast.StringNode - project *ast.StringNode + nodes []ast.Node + query *ast.GroupNode + queries int + stray bool } const ( @@ -75,13 +73,8 @@ func (t *QueryTemplate) findMarkers(nodes []ast.Node) { } t.findMarkers(node.Nodes) case *ast.StringNode: - switch node.Value { - case queryMarker: + if node.Value == queryMarker { t.stray = true - case personalRootMarker: - t.personal = node - case projectRootMarker: - t.project = node } } } @@ -102,20 +95,25 @@ func (t *QueryTemplate) Expand(query *ast.Ast, space *provider.StorageSpace) *as return config.NoResourceID } + // the user's nodes are copied without replace, so a marker in the user's + // query stays what it is return &ast.Ast{Nodes: cloneNodes(t.nodes, func(n ast.Node) ast.Node { - switch n { - case t.query: + if n == ast.Node(t.query) { c := *t.query c.Nodes = cloneNodes(query.Nodes, nil) return &c - case t.personal: - c := *t.personal - c.Value = rootID(_spaceTypePersonal) - return &c - case t.project: - c := *t.project - c.Value = rootID(_spaceTypeProject) - return &c + } + if s, ok := n.(*ast.StringNode); ok { + switch s.Value { + case personalRootMarker: + c := *s + c.Value = rootID(_spaceTypePersonal) + return &c + case projectRootMarker: + c := *s + c.Value = rootID(_spaceTypeProject) + return &c + } } return nil })} diff --git a/services/search/pkg/search/template_test.go b/services/search/pkg/search/template_test.go index 494f5508d7..8cb7184727 100644 --- a/services/search/pkg/search/template_test.go +++ b/services/search/pkg/search/template_test.go @@ -95,6 +95,23 @@ var _ = Describe("QueryTemplate", func() { Expect(render(expanded.Nodes)).To(Equal(render(parse(`(NOT mediatype:folder AND (name:x OR name:y)) OR tag:keep`).Nodes))) }) + It("fills a root placeholder that is used twice", func() { + t, err := search.ParseQueryTemplate(`({{.QUERY}}) AND NOT id:{{.SPACE_ROOT_PERSONAL}} AND NOT parentid:{{.SPACE_ROOT_PERSONAL}}`) + Expect(err).ToNot(HaveOccurred()) + space := &sprovider.StorageSpace{SpaceType: "personal", Root: &sprovider.ResourceId{StorageId: "1", SpaceId: "2", OpaqueId: "2"}} + + expanded := t.Expand(parse(`name:x`), space) + Expect(render(expanded.Nodes)).To(Equal(render(parse(`(name:x) AND NOT id:"1$2!2" AND NOT parentid:"1$2!2"`).Nodes))) + }) + + It("leaves a marker in the user's query alone", func() { + t, err := search.ParseQueryTemplate(`({{.QUERY}}) AND NOT id:{{.SPACE_ROOT_PERSONAL}}`) + Expect(err).ToNot(HaveOccurred()) + + expanded := t.Expand(parse(`name:opencloudpersonalrootmarker`), &sprovider.StorageSpace{}) + Expect(render(expanded.Nodes)).To(Equal(render(parse(`(name:opencloudpersonalrootmarker) AND NOT id:"none$none!none"`).Nodes))) + }) + It("keeps the key of the query group", func() { t, err := search.ParseQueryTemplate(`name:({{.QUERY}})`) Expect(err).ToNot(HaveOccurred())