Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ struct S3ObjectStorageConfig {

bool Validate() const;
static S3ObjectStorageConfig FromEnvironment();
// Drops trailing '/' so "p", "p/" and "p///" name the same namespace.
// Every physical key is built as key_prefix + "/<area>/..."; a trailing
// slash would make LIST query "p//objects/" while PUT/GET land on the
// endpoint-normalized "p/objects/", hiding every object from ScanMeta.
static std::string NormalizeKeyPrefix(std::string prefix);
};

#ifdef HAVE_AWS_SDK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,15 @@ S3ObjectStorageConfig S3ObjectStorageConfig::FromEnvironment() {
read("MOONCAKE_AWS_BUCKET_NAME", config.bucket);
read("MOONCAKE_AWS_REGION", config.region);
read("MOONCAKE_S3_KEY_PREFIX", config.key_prefix);
config.key_prefix = NormalizeKeyPrefix(std::move(config.key_prefix));
return config;
}

std::string S3ObjectStorageConfig::NormalizeKeyPrefix(std::string prefix) {
while (!prefix.empty() && prefix.back() == '/') prefix.pop_back();
return prefix;
}

bool S3ObjectStorageConfig::Validate() const {
// A durable namespace needs an explicit, credential-free HTTP(S) origin.
// Reject it before any SDK request rather than mounting an unusable Store.
Expand All @@ -33,7 +39,9 @@ bool S3ObjectStorageConfig::Validate() const {
const auto authority = endpoint.substr(scheme_end + 3);
if (authority.empty() || authority.find('/') != std::string::npos)
return false;
return !bucket.empty() && !region.empty() && !key_prefix.empty() &&
// A prefix of only slashes normalizes to empty: no owned namespace.
return !bucket.empty() && !region.empty() &&
!NormalizeKeyPrefix(key_prefix).empty() &&
key_prefix.size() <= 909 &&
key_prefix.find('\0') == std::string::npos &&
bucket.find('\0') == std::string::npos &&
Expand Down Expand Up @@ -70,8 +78,19 @@ int Unhex(char c) {
}
} // namespace

namespace {
// Configs built in code skip FromEnvironment; normalize on construction too so
// PhysicalPrefix/WriterPrefix/DeletionIntentKey and the published durable
// namespace always share one canonical prefix.
S3ObjectStorageConfig Normalized(S3ObjectStorageConfig config) {
config.key_prefix =
S3ObjectStorageConfig::NormalizeKeyPrefix(std::move(config.key_prefix));
return config;
}
} // namespace

S3ObjectStorageAdapter::S3ObjectStorageAdapter(S3ObjectStorageConfig config)
: config_(std::move(config)) {}
: config_(Normalized(std::move(config))) {}

S3ObjectStorageAdapter::~S3ObjectStorageAdapter() {
helper_.reset();
Expand Down
31 changes: 31 additions & 0 deletions mooncake-store/tests/object_storage_adapter_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <algorithm>
#include <array>
#include <cstdlib>
#include <cstring>
#include <filesystem>
#include <limits>
Expand Down Expand Up @@ -731,4 +732,34 @@ TEST(S3ObjectStorageConfigTest, RequiresExplicitCredentialFreeOrigin) {
EXPECT_TRUE(config.Validate()) << endpoint;
}
}

// mint#1775: a trailing '/' on MOONCAKE_S3_KEY_PREFIX made ListKeys/ScanMeta
// query "p//objects/" while PUT/GET used the endpoint-normalized
// "p/objects/", so no provider ever re-registered an offloaded object.
TEST(S3ObjectStorageConfigTest, KeyPrefixTrailingSlashesNameOneNamespace) {
for (const std::string prefix : {"p", "p/", "p///"}) {
EXPECT_EQ(S3ObjectStorageConfig::NormalizeKeyPrefix(prefix), "p")
<< prefix;
}
EXPECT_EQ(S3ObjectStorageConfig::NormalizeKeyPrefix("a/b/"), "a/b");
EXPECT_EQ(S3ObjectStorageConfig::NormalizeKeyPrefix("/"), "");

ASSERT_EQ(setenv("MOONCAKE_S3_KEY_PREFIX", "mint-run-20261024//", 1), 0);
const auto config = S3ObjectStorageConfig::FromEnvironment();
ASSERT_EQ(unsetenv("MOONCAKE_S3_KEY_PREFIX"), 0);
EXPECT_EQ(config.key_prefix, "mint-run-20261024");
}

TEST(S3ObjectStorageConfigTest, RejectsPrefixThatIsOnlySlashes) {
S3ObjectStorageConfig config;
config.endpoint = "https://oss.example.test";
config.bucket = "bucket";
config.region = "region";
for (const std::string prefix : {"/", "///"}) {
config.key_prefix = prefix;
EXPECT_FALSE(config.Validate()) << prefix;
}
config.key_prefix = "isolated-prefix/";
EXPECT_TRUE(config.Validate());
}
} // namespace mooncake
Loading