From 4443254fb92710c31b1830bc13d3d642f3c2917f Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 18 Mar 2021 14:52:49 +0200 Subject: [PATCH] lib/storage: prevent from infinite loop if `{__graphite__="..."}` filter matches a metric name with `*`, `[` or `{` chars The idea has been borrowed from https://github.com/VictoriaMetrics/VictoriaMetrics/pull/1137 --- docs/CHANGELOG.md | 2 ++ lib/storage/storage.go | 41 +++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ada4f3bf6e..89f1f05df4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,8 @@ * `process_resident_memory_peak_bytes` - peak RSS usage for the process. * `process_virtual_memory_peak_bytes` - peak virtual memory usage for the process. +* BUGFIX: prevent from infinite loop on `{__graphite__="..."}` filters when a metric name contains `*`, `{` or `[` chars. + # [v1.56.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.56.0) diff --git a/lib/storage/storage.go b/lib/storage/storage.go index 146310ceea..a1aa646781 100644 --- a/lib/storage/storage.go +++ b/lib/storage/storage.go @@ -1,6 +1,7 @@ package storage import ( + "bytes" "fmt" "io" "io/ioutil" @@ -15,6 +16,7 @@ import ( "time" "unsafe" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/bytesutil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/cgroup" "github.com/VictoriaMetrics/VictoriaMetrics/lib/encoding" "github.com/VictoriaMetrics/VictoriaMetrics/lib/fasttime" @@ -1122,14 +1124,16 @@ func (s *Storage) SearchTagValueSuffixes(accountID, projectID uint32, tr TimeRan } // SearchGraphitePaths returns all the matching paths for the given graphite query on the given tr. -// -// If more than maxPaths paths is found, then only the first maxPaths paths is returned. func (s *Storage) SearchGraphitePaths(accountID, projectID uint32, tr TimeRange, query []byte, maxPaths int, deadline uint64) ([]string, error) { - queryStr := string(query) - n := strings.IndexAny(queryStr, "*[{") + return s.searchGraphitePaths(accountID, projectID, tr, nil, query, maxPaths, deadline) +} + +func (s *Storage) searchGraphitePaths(accountID, projectID uint32, tr TimeRange, qHead, qTail []byte, maxPaths int, deadline uint64) ([]string, error) { + n := strings.IndexAny(bytesutil.ToUnsafeString(qTail), "*[{") if n < 0 { - // Verify that the query matches a metric name. - suffixes, err := s.SearchTagValueSuffixes(accountID, projectID, tr, nil, query, '.', 1, deadline) + // Verify that qHead matches a metric name. + qHead = append(qHead, qTail...) + suffixes, err := s.SearchTagValueSuffixes(accountID, projectID, tr, nil, qHead, '.', 1, deadline) if err != nil { return nil, err } @@ -1141,9 +1145,10 @@ func (s *Storage) SearchGraphitePaths(accountID, projectID uint32, tr TimeRange, // The query matches a metric name with additional suffix. return nil, nil } - return []string{queryStr}, nil + return []string{string(qHead)}, nil } - suffixes, err := s.SearchTagValueSuffixes(accountID, projectID, tr, nil, query[:n], '.', maxPaths, deadline) + qHead = append(qHead, qTail[:n]...) + suffixes, err := s.SearchTagValueSuffixes(accountID, projectID, tr, nil, qHead, '.', maxPaths, deadline) if err != nil { return nil, err } @@ -1153,34 +1158,34 @@ func (s *Storage) SearchGraphitePaths(accountID, projectID uint32, tr TimeRange, if len(suffixes) >= maxPaths { return nil, fmt.Errorf("more than maxPaths=%d suffixes found", maxPaths) } - qPrefixStr := queryStr[:n] - qTail := "" - qNode := queryStr[n:] + qNode := qTail[n:] + qTail = nil mustMatchLeafs := true - if m := strings.IndexByte(qNode, '.'); m >= 0 { + if m := bytes.IndexByte(qNode, '.'); m >= 0 { qTail = qNode[m+1:] qNode = qNode[:m+1] mustMatchLeafs = false } - re, err := getRegexpForGraphiteQuery(qNode) + re, err := getRegexpForGraphiteQuery(string(qNode)) if err != nil { return nil, err } + qHeadLen := len(qHead) var paths []string for _, suffix := range suffixes { if len(paths) > maxPaths { - paths = paths[:maxPaths] - break + return nil, fmt.Errorf("more than maxPath=%d paths found", maxPaths) } if !re.MatchString(suffix) { continue } if mustMatchLeafs { - paths = append(paths, qPrefixStr+suffix) + qHead = append(qHead[:qHeadLen], suffix...) + paths = append(paths, string(qHead)) continue } - q := qPrefixStr + suffix + qTail - ps, err := s.SearchGraphitePaths(accountID, projectID, tr, []byte(q), maxPaths, deadline) + qHead = append(qHead[:qHeadLen], suffix...) + ps, err := s.searchGraphitePaths(accountID, projectID, tr, qHead, qTail, maxPaths, deadline) if err != nil { return nil, err }