Skip to content

Commit bb486c0

Browse files
xiaocai2333xiaocai2333
authored
fix: Fix path concatenation error when rootPath = "." in minio (#46220)
issue: #46219 --------- Signed-off-by: Cai Zhang <[email protected]>
1 parent 3f063a2 commit bb486c0

File tree

3 files changed

+96
-9
lines changed

3 files changed

+96
-9
lines changed

‎internal/core/src/storage/FileManager.h‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@
3131

3232
namespace milvus::storage {
3333

34+
// Normalize path to be consistent with Go's path.Join behavior.
35+
// This handles two issues:
36+
// 1. Removes leading "./" when root_path is "."
37+
// 2. Removes trailing "/." that lexically_normal() may produce
38+
inline std::string
39+
NormalizePath(const boost::filesystem::path& path) {
40+
auto result = path.lexically_normal().string();
41+
// Remove trailing "/." if present
42+
if (result.size() >= 2 && result.substr(result.size() - 2) == "/.") {
43+
result = result.substr(0, result.size() - 1);
44+
}
45+
return result;
46+
}
47+
3448
struct FileManagerContext {
3549
FileManagerContext() : chunkManagerPtr(nullptr) {
3650
}
@@ -179,7 +193,7 @@ class FileManagerImpl : public milvus::FileManager {
179193
std::to_string(index_meta_.index_version) + "/" +
180194
std::to_string(field_meta_.partition_id) + "/" +
181195
std::to_string(field_meta_.segment_id);
182-
return (prefix / path / path1).string();
196+
return NormalizePath(prefix / path / path1);
183197
}
184198

185199
virtual std::string
@@ -198,7 +212,7 @@ class FileManagerImpl : public milvus::FileManager {
198212
if (bucket.empty()) {
199213
return v1_prefix;
200214
} else {
201-
return (bucket / v1_prefix).string();
215+
return NormalizePath(bucket / v1_prefix);
202216
}
203217
}
204218

@@ -213,7 +227,7 @@ class FileManagerImpl : public milvus::FileManager {
213227
std::to_string(field_meta_.partition_id) + "/" +
214228
std::to_string(field_meta_.segment_id) + "/" +
215229
std::to_string(field_meta_.field_id);
216-
return (prefix / path / path1).string();
230+
return NormalizePath(prefix / path / path1);
217231
}
218232

219233
protected:

‎internal/core/src/storage/Util.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ GenIndexPathPrefixByType(ChunkManagerPtr cm,
713713
boost::filesystem::path path = std::string(index_type);
714714
boost::filesystem::path path1 =
715715
GenIndexPathIdentifier(build_id, index_version, segment_id, field_id);
716-
return (prefix / path / path1).string();
716+
return NormalizePath(prefix / path / path1);
717717
}
718718

719719
std::string
@@ -749,7 +749,7 @@ GenJsonStatsPathPrefix(ChunkManagerPtr cm,
749749
boost::filesystem::path path1 =
750750
GenIndexPathIdentifier(build_id, index_version, segment_id, field_id);
751751

752-
return (prefix / path / path1).string();
752+
return NormalizePath(prefix / path / path1);
753753
}
754754

755755
std::string
@@ -764,7 +764,7 @@ GenJsonStatsPathIdentifier(int64_t build_id,
764764
std::to_string(index_version) / std::to_string(collection_id) /
765765
std::to_string(partition_id) / std::to_string(segment_id) /
766766
std::to_string(field_id);
767-
return p.string() + "/";
767+
return NormalizePath(p);
768768
}
769769

770770
std::string
@@ -784,7 +784,7 @@ GenRemoteJsonStatsPathPrefix(ChunkManagerPtr cm,
784784
partition_id,
785785
segment_id,
786786
field_id);
787-
return p.string();
787+
return NormalizePath(p);
788788
}
789789

790790
std::string
@@ -811,15 +811,15 @@ GenFieldRawDataPathPrefix(ChunkManagerPtr cm,
811811
boost::filesystem::path path = std::string(RAWDATA_ROOT_PATH);
812812
boost::filesystem::path path1 =
813813
std::to_string(segment_id) + "/" + std::to_string(field_id) + "/";
814-
return (prefix / path / path1).string();
814+
return NormalizePath(prefix / path / path1);
815815
}
816816

817817
std::string
818818
GetSegmentRawDataPathPrefix(ChunkManagerPtr cm, int64_t segment_id) {
819819
boost::filesystem::path prefix = cm->GetRootPath();
820820
boost::filesystem::path path = std::string(RAWDATA_ROOT_PATH);
821821
boost::filesystem::path path1 = std::to_string(segment_id);
822-
return (prefix / path / path1).string();
822+
return NormalizePath(prefix / path / path1);
823823
}
824824

825825
std::pair<std::string, size_t>

‎internal/core/unittest/test_storage.cpp‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <string>
1717
#include <vector>
1818
#include "common/EasyAssert.h"
19+
#include "storage/FileManager.h"
1920
#include "storage/LocalChunkManagerSingleton.h"
2021
#include "storage/RemoteChunkManagerSingleton.h"
2122
#include "storage/Util.h"
@@ -241,4 +242,76 @@ TEST_F(StorageUtilTest, TestInitArrowFileSystem) {
241242
// auto fs = InitArrowFileSystem(remote_config);
242243
// ASSERT_NE(fs, nullptr);
243244
// }
245+
}
246+
247+
// Test cases for NormalizePath function
248+
// NormalizePath uses boost::filesystem::path::lexically_normal() and then
249+
// removes trailing "/." (only the dot, keeping the slash)
250+
TEST_F(StorageUtilTest, NormalizePath) {
251+
// === Basic paths ===
252+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/c")), "a/b/c");
253+
EXPECT_EQ(NormalizePath(boost::filesystem::path("")), "");
254+
EXPECT_EQ(NormalizePath(boost::filesystem::path("file")), "file");
255+
256+
// === Dot handling ===
257+
EXPECT_EQ(NormalizePath(boost::filesystem::path(".")), ".");
258+
EXPECT_EQ(NormalizePath(boost::filesystem::path("./a/b")), "a/b");
259+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/./b")), "a/b");
260+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/.")), "a/b/");
261+
EXPECT_EQ(NormalizePath(boost::filesystem::path("./a/./b/.")), "a/b/");
262+
263+
// === Double dot (..) handling ===
264+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/../c")), "a/c");
265+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/c/../../d")), "a/d");
266+
EXPECT_EQ(NormalizePath(boost::filesystem::path("../a/b")), "../a/b");
267+
268+
// === Trailing slash ===
269+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/")), "a/b/");
270+
EXPECT_EQ(NormalizePath(boost::filesystem::path("files/")), "files/");
271+
272+
// === Absolute paths ===
273+
EXPECT_EQ(NormalizePath(boost::filesystem::path("/a/b/c")), "/a/b/c");
274+
EXPECT_EQ(NormalizePath(boost::filesystem::path("/a/./b")), "/a/b");
275+
EXPECT_EQ(NormalizePath(boost::filesystem::path("/a/b/.")), "/a/b/");
276+
EXPECT_EQ(NormalizePath(boost::filesystem::path("/")), "/");
277+
278+
// === Multiple slashes ===
279+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a//b//c")), "a/b/c");
280+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/./b//c")), "a/b/c");
281+
282+
// === Real-world scenarios (S3/MinIO) ===
283+
EXPECT_EQ(NormalizePath(boost::filesystem::path("bucket/index_files/123")),
284+
"bucket/index_files/123");
285+
// Key fix for 403 error
286+
EXPECT_EQ(
287+
NormalizePath(boost::filesystem::path("./index_files/segment_123")),
288+
"index_files/segment_123");
289+
290+
// Path construction with root_path = "."
291+
boost::filesystem::path prefix = ".";
292+
boost::filesystem::path path = "index_files";
293+
boost::filesystem::path path1 = "segment_123";
294+
EXPECT_EQ(NormalizePath(prefix / path / path1), "index_files/segment_123");
295+
296+
// Non-empty root path
297+
boost::filesystem::path prefix2 = "files";
298+
EXPECT_EQ(NormalizePath(prefix2 / path / path1),
299+
"files/index_files/segment_123");
300+
301+
// Root path with trailing slash
302+
boost::filesystem::path prefix3 = "files/";
303+
EXPECT_EQ(NormalizePath(prefix3 / path / path1),
304+
"files/index_files/segment_123");
305+
306+
// Empty root path
307+
boost::filesystem::path prefix4 = "";
308+
EXPECT_EQ(NormalizePath(prefix4 / path / path1), "index_files/segment_123");
309+
310+
// === Edge cases ===
311+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b/..")), "a");
312+
EXPECT_EQ(NormalizePath(boost::filesystem::path("./a/../b/./c/../d")),
313+
"b/d");
314+
EXPECT_EQ(NormalizePath(boost::filesystem::path("a/b c/d")), "a/b c/d");
315+
EXPECT_EQ(NormalizePath(boost::filesystem::path("./.")), ".");
316+
EXPECT_EQ(NormalizePath(boost::filesystem::path("./..")), "..");
244317
}

0 commit comments

Comments
 (0)