Skip to content

Commit 560d375

Browse files
committed
Fix false positive in isAbsPathOverlapping path boundary check
strings.HasPrefix without a trailing "/" causes false positives when one path is a string prefix of another but not a path ancestor (e.g. "/data/tsdb" vs "/data/tsdb-compactor/cache"). Append "/" before comparing to enforce path segment boundaries.
1 parent f0c965c commit 560d375

File tree

3 files changed

+21
-1
lines changed

3 files changed

+21
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
* [ENHANCEMENT] Ingester: Add experimental file based Kafka consumer group offset tracking via flag `-ingest-storage.kafka.consumer-group-offset-commit-file-enforced`. #14110
130130
* [ENHANCEMENT] Store-gateway: Add "OOO" column to the tenant blocks page to indicate whether each block was created from out-of-order samples. #14283
131131
* [ENHANCEMENT] Ingester: Optimize ingestion from Kafka in clusters with mixed size tenants. #13924 #13961 #14302
132+
* [BUGFIX] Mimir: Fix false positive in filesystem path overlap detection when one path is a string prefix of another but not an ancestor directory (e.g. `/data/tsdb` vs `/data/tsdb-compactor/cache`). #14426
132133
* [BUGFIX] Mimir: Fix nil pointer dereference when `-target` is set to an empty string. #14381
133134
* [BUGFIX] API: Fixed web UI links not respecting `-server.path-prefix` configuration. #14090
134135
* [BUGFIX] Distributor: Fix issue where distributors didn't send custom values of native histograms. #13849

pkg/mimir/mimir.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,9 @@ func isAbsPathOverlapping(firstAbsPath, secondAbsPath string) bool {
642642
}
643643

644644
// The base directories are different, but they could still overlap if one is the child of the other one.
645-
return strings.HasPrefix(firstAbsPath, secondAbsPath) || strings.HasPrefix(secondAbsPath, firstAbsPath)
645+
// We append "/" to ensure we match on path segment boundaries and avoid false positives
646+
// like "/data/tsdb" matching "/data/tsdb-compactor/cache".
647+
return strings.HasPrefix(firstAbsPath, secondAbsPath+"/") || strings.HasPrefix(secondAbsPath, firstAbsPath+"/")
646648
}
647649

648650
func (c *Config) registerServerFlagsWithChangedDefaultValues(fs *flag.FlagSet) {

pkg/mimir/mimir_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,23 @@ func TestIsAbsPathOverlapping(t *testing.T) {
807807
second: "/path/to/more/data",
808808
expected: false,
809809
},
810+
// Paths at different depths where one is a string prefix of the other
811+
// but NOT a path ancestor (regression test for path boundary check).
812+
{
813+
first: "/data/tsdb",
814+
second: "/data/tsdb-compactor/cache",
815+
expected: false,
816+
},
817+
{
818+
first: "/data/tsdb-compactor/cache",
819+
second: "/data/tsdb",
820+
expected: false,
821+
},
822+
{
823+
first: "/data/tsdb",
824+
second: "/data/tsdb/wal",
825+
expected: true,
826+
},
810827
}
811828

812829
for _, testData := range tests {

0 commit comments

Comments
 (0)