Skip to content

Commit 84ecda3

Browse files
jyxjjjxrgzscodex
committed
fix(search): apply access filtering before paginating results
* fix: replace strings.HasPrefix with utils.IsSubPath for path validation Signed-off-by: MadDogOwner <xiaoran@xrgzs.top> * fix: re-validate shared paths to ensure they remain within the creator's base path Signed-off-by: MadDogOwner <xiaoran@xrgzs.top> --------- Signed-off-by: MadDogOwner <xiaoran@xrgzs.top> Co-authored-by: MadDogOwner <xiaoran@xrgzs.top> * fix(search): apply access filtering before paginating results Co-authored-by: Codex <267193182+codex@users.noreply.github.com> Signed-off-by: MadDogOwner <xiaoran@xrgzs.top> --------- Signed-off-by: MadDogOwner <xiaoran@xrgzs.top> Signed-off-by: jyxjjj <16695261+jyxjjj@users.noreply.github.com> Co-authored-by: MadDogOwner <xiaoran@xrgzs.top> Co-authored-by: Codex <267193182+codex@users.noreply.github.com>
1 parent 2deb16b commit 84ecda3

6 files changed

Lines changed: 239 additions & 32 deletions

File tree

‎internal/search/bleve/search.go‎

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,9 @@ func (b *Bleve) Config() searcher.Config {
2626
}
2727

2828
func (b *Bleve) Search(ctx context.Context, req model.SearchReq) ([]model.SearchNode, int64, error) {
29-
var queries []query2.Query
30-
query := bleve.NewMatchQuery(req.Keywords)
31-
query.SetField("name")
32-
queries = append(queries, query)
33-
if req.Scope != 0 {
34-
isDir := req.Scope == 1
35-
isDirQuery := bleve.NewBoolFieldQuery(isDir)
36-
queries = append(queries, isDirQuery)
37-
}
38-
reqQuery := bleve.NewConjunctionQuery(queries...)
29+
reqQuery := buildQuery(req)
3930
search := bleve.NewSearchRequest(reqQuery)
40-
search.SortBy([]string{"name"})
31+
search.SortBy([]string{"name", "_id"})
4132
search.From = (req.Page - 1) * req.PerPage
4233
search.Size = req.PerPage
4334
search.Fields = []string{"*"}
@@ -47,14 +38,74 @@ func (b *Bleve) Search(ctx context.Context, req model.SearchReq) ([]model.Search
4738
return nil, 0, err
4839
}
4940
res, err := utils.SliceConvert(searchResults.Hits, func(src *search2.DocumentMatch) (model.SearchNode, error) {
50-
return model.SearchNode{
51-
Parent: src.Fields["parent"].(string),
52-
Name: src.Fields["name"].(string),
53-
IsDir: src.Fields["is_dir"].(bool),
54-
Size: int64(src.Fields["size"].(float64)),
55-
}, nil
41+
return searchNodeFromHit(src), nil
5642
})
57-
return res, int64(searchResults.Total), nil
43+
return res, int64(searchResults.Total), err
44+
}
45+
46+
const searchBatchSize = 1000
47+
48+
func (b *Bleve) SearchFiltered(ctx context.Context, req model.SearchReq, filter searcher.Filter) ([]model.SearchNode, int64, error) {
49+
reqQuery := buildQuery(req)
50+
from := int64(req.Page-1) * int64(req.PerPage)
51+
to := from + int64(req.PerPage)
52+
var (
53+
result []model.SearchNode
54+
total int64
55+
searchAfter []string
56+
)
57+
for {
58+
search := bleve.NewSearchRequest(reqQuery)
59+
search.SortBy([]string{"name", "_id"})
60+
search.Size = searchBatchSize
61+
search.Fields = []string{"*"}
62+
if searchAfter != nil {
63+
search.SetSearchAfter(searchAfter)
64+
}
65+
searchResults, err := b.BIndex.Search(search)
66+
if err != nil {
67+
log.Errorf("search error: %+v", err)
68+
return nil, 0, err
69+
}
70+
for _, hit := range searchResults.Hits {
71+
node := searchNodeFromHit(hit)
72+
if !utils.IsSubPath(req.Parent, node.Parent) || filter != nil && !filter(node) {
73+
continue
74+
}
75+
if total >= from && total < to {
76+
result = append(result, node)
77+
}
78+
total++
79+
}
80+
if len(searchResults.Hits) < searchBatchSize {
81+
break
82+
}
83+
last := searchResults.Hits[len(searchResults.Hits)-1]
84+
searchAfter = append(searchAfter[:0], last.Sort...)
85+
}
86+
return result, total, nil
87+
}
88+
89+
func buildQuery(req model.SearchReq) query2.Query {
90+
var queries []query2.Query
91+
query := bleve.NewMatchQuery(req.Keywords)
92+
query.SetField("name")
93+
queries = append(queries, query)
94+
if req.Scope != 0 {
95+
isDir := req.Scope == 1
96+
isDirQuery := bleve.NewBoolFieldQuery(isDir)
97+
queries = append(queries, isDirQuery)
98+
}
99+
return bleve.NewConjunctionQuery(queries...)
100+
}
101+
102+
func searchNodeFromHit(src *search2.DocumentMatch) model.SearchNode {
103+
return model.SearchNode{
104+
Parent: src.Fields["parent"].(string),
105+
Name: src.Fields["name"].(string),
106+
IsDir: src.Fields["is_dir"].(bool),
107+
Size: int64(src.Fields["size"].(float64)),
108+
}
58109
}
59110

60111
func (b *Bleve) Index(ctx context.Context, node model.SearchNode) error {
@@ -103,3 +154,4 @@ func (b *Bleve) Clear(ctx context.Context) error {
103154
}
104155

105156
var _ searcher.Searcher = (*Bleve)(nil)
157+
var _ searcher.FilteredSearcher = (*Bleve)(nil)

‎internal/search/bleve/search_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package bleve
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/OpenListTeam/OpenList/v4/internal/model"
9+
blevelib "github.com/blevesearch/bleve/v2"
10+
)
11+
12+
func TestSearchFilteredKeepsDuplicateSortValuesAcrossBatches(t *testing.T) {
13+
indexMapping := blevelib.NewIndexMapping()
14+
searchNodeMapping := blevelib.NewDocumentMapping()
15+
searchNodeMapping.AddFieldMappingsAt("is_dir", blevelib.NewBooleanFieldMapping())
16+
searchNodeMapping.AddFieldMappingsAt("parent", blevelib.NewTextFieldMapping())
17+
searchNodeMapping.AddFieldMappingsAt("name", blevelib.NewKeywordFieldMapping())
18+
indexMapping.AddDocumentMapping("SearchNode", searchNodeMapping)
19+
index, err := blevelib.NewMemOnly(indexMapping)
20+
if err != nil {
21+
t.Fatalf("NewMemOnly() error = %v", err)
22+
}
23+
t.Cleanup(func() { _ = index.Close() })
24+
25+
batch := index.NewBatch()
26+
for i := 0; i < searchBatchSize+1; i++ {
27+
batch.Index(fmt.Sprintf("allowed-%04d", i), model.SearchNode{
28+
Parent: "/base",
29+
Name: "duplicate",
30+
})
31+
}
32+
batch.Index("denied", model.SearchNode{Parent: "/base2", Name: "duplicate"})
33+
if err := index.Batch(batch); err != nil {
34+
t.Fatalf("Batch() error = %v", err)
35+
}
36+
37+
b := &Bleve{BIndex: index}
38+
nodes, total, err := b.SearchFiltered(context.Background(), model.SearchReq{
39+
Parent: "/base",
40+
Keywords: "duplicate",
41+
PageReq: model.PageReq{Page: 1, PerPage: searchBatchSize + 1},
42+
}, nil)
43+
if err != nil {
44+
t.Fatalf("SearchFiltered() error = %v", err)
45+
}
46+
if total != searchBatchSize+1 {
47+
t.Fatalf("SearchFiltered() total = %d, want %d", total, searchBatchSize+1)
48+
}
49+
if len(nodes) != searchBatchSize+1 {
50+
t.Fatalf("SearchFiltered() returned %d nodes, want %d", len(nodes), searchBatchSize+1)
51+
}
52+
}

‎internal/search/search.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,46 @@ func Search(ctx context.Context, req model.SearchReq) ([]model.SearchNode, int64
5151
return instance.Search(ctx, req)
5252
}
5353

54+
const searchBatchSize = 1000
55+
56+
func SearchFiltered(ctx context.Context, req model.SearchReq, filter searcher.Filter) ([]model.SearchNode, int64, error) {
57+
if filteredSearcher, ok := instance.(searcher.FilteredSearcher); ok {
58+
return filteredSearcher.SearchFiltered(ctx, req, filter)
59+
}
60+
61+
batchReq := req
62+
batchReq.Page = 1
63+
batchReq.PerPage = searchBatchSize
64+
from := int64(req.Page-1) * int64(req.PerPage)
65+
to := from + int64(req.PerPage)
66+
var (
67+
result []model.SearchNode
68+
filteredTotal int64
69+
processedTotal int64
70+
)
71+
for {
72+
nodes, total, err := instance.Search(ctx, batchReq)
73+
if err != nil {
74+
return nil, 0, err
75+
}
76+
for _, node := range nodes {
77+
if filter != nil && !filter(node) {
78+
continue
79+
}
80+
if filteredTotal >= from && filteredTotal < to {
81+
result = append(result, node)
82+
}
83+
filteredTotal++
84+
}
85+
processedTotal += int64(len(nodes))
86+
if len(nodes) == 0 || processedTotal >= total {
87+
break
88+
}
89+
batchReq.Page++
90+
}
91+
return result, filteredTotal, nil
92+
}
93+
5494
func Index(ctx context.Context, parent string, obj model.Obj) error {
5595
if instance == nil {
5696
return errs.SearchNotAvailable

‎internal/search/search_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package search
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/OpenListTeam/OpenList/v4/internal/model"
8+
"github.com/OpenListTeam/OpenList/v4/internal/search/searcher"
9+
)
10+
11+
type filteredSearchStub struct {
12+
nodes []model.SearchNode
13+
}
14+
15+
func (s *filteredSearchStub) Config() searcher.Config {
16+
return searcher.Config{Name: "stub"}
17+
}
18+
19+
func (s *filteredSearchStub) Search(_ context.Context, req model.SearchReq) ([]model.SearchNode, int64, error) {
20+
from := (req.Page - 1) * req.PerPage
21+
if from >= len(s.nodes) {
22+
return nil, int64(len(s.nodes)), nil
23+
}
24+
to := min(from+req.PerPage, len(s.nodes))
25+
return s.nodes[from:to], int64(len(s.nodes)), nil
26+
}
27+
28+
func (s *filteredSearchStub) Index(context.Context, model.SearchNode) error { return nil }
29+
func (s *filteredSearchStub) BatchIndex(context.Context, []model.SearchNode) error { return nil }
30+
func (s *filteredSearchStub) Get(context.Context, string) ([]model.SearchNode, error) {
31+
return nil, nil
32+
}
33+
func (s *filteredSearchStub) Del(context.Context, string) error { return nil }
34+
func (s *filteredSearchStub) Release(context.Context) error { return nil }
35+
func (s *filteredSearchStub) Clear(context.Context) error { return nil }
36+
37+
func TestSearchFilteredFiltersBeforePagination(t *testing.T) {
38+
previous := instance
39+
instance = &filteredSearchStub{nodes: []model.SearchNode{
40+
{Name: "denied-1"},
41+
{Name: "allowed-1"},
42+
{Name: "denied-2"},
43+
{Name: "allowed-2"},
44+
}}
45+
t.Cleanup(func() { instance = previous })
46+
47+
nodes, total, err := SearchFiltered(context.Background(), model.SearchReq{
48+
PageReq: model.PageReq{Page: 2, PerPage: 1},
49+
}, func(node model.SearchNode) bool {
50+
return node.Name == "allowed-1" || node.Name == "allowed-2"
51+
})
52+
if err != nil {
53+
t.Fatalf("SearchFiltered() error = %v", err)
54+
}
55+
if total != 2 {
56+
t.Fatalf("SearchFiltered() total = %d, want 2", total)
57+
}
58+
if len(nodes) != 1 || nodes[0].Name != "allowed-2" {
59+
t.Fatalf("SearchFiltered() nodes = %#v, want allowed-2", nodes)
60+
}
61+
}

‎internal/search/searcher/searcher.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,13 @@ type Config struct {
1111
AutoUpdate bool
1212
}
1313

14+
type Filter func(node model.SearchNode) bool
15+
16+
// FilteredSearcher applies filtering before calculating pagination and totals.
17+
type FilteredSearcher interface {
18+
SearchFiltered(ctx context.Context, req model.SearchReq, filter Filter) ([]model.SearchNode, int64, error)
19+
}
20+
1421
type Searcher interface {
1522
// Config of the searcher
1623
Config() Config

‎server/handles/search.go‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,27 +43,22 @@ func Search(c *gin.Context) {
4343
common.ErrorResp(c, err, 400)
4444
return
4545
}
46-
nodes, total, err := search.Search(c, req.SearchReq)
47-
if err != nil {
48-
common.ErrorResp(c, err, 500)
49-
return
50-
}
51-
var filteredNodes []model.SearchNode
52-
for _, node := range nodes {
46+
nodes, total, err := search.SearchFiltered(c, req.SearchReq, func(node model.SearchNode) bool {
5347
if !utils.IsSubPath(user.BasePath, node.Parent) {
54-
continue
48+
return false
5549
}
5650
meta, err := op.GetNearestMeta(node.Parent)
5751
if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) {
58-
continue
59-
}
60-
if !common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password) {
61-
continue
52+
return false
6253
}
63-
filteredNodes = append(filteredNodes, node)
54+
return common.CanAccess(user, meta, path.Join(node.Parent, node.Name), req.Password)
55+
})
56+
if err != nil {
57+
common.ErrorResp(c, err, 500)
58+
return
6459
}
6560
common.SuccessResp(c, common.PageResp{
66-
Content: utils.MustSliceConvert(filteredNodes, nodeToSearchResp),
61+
Content: utils.MustSliceConvert(nodes, nodeToSearchResp),
6762
Total: total,
6863
})
6964
}

0 commit comments

Comments
 (0)