diff --git a/src/iceberg/catalog/rest/resource_paths.cc b/src/iceberg/catalog/rest/resource_paths.cc index 354400c4d..d18dd4636 100644 --- a/src/iceberg/catalog/rest/resource_paths.cc +++ b/src/iceberg/catalog/rest/resource_paths.cc @@ -34,6 +34,8 @@ Result> ResourcePaths::Make( if (base_uri.empty()) { return InvalidArgument("Base URI is empty"); } + ICEBERG_PRECHECK(!namespace_separator.empty(), + "REST namespace separator cannot be empty"); return std::unique_ptr( new ResourcePaths(std::move(base_uri), prefix, namespace_separator)); } diff --git a/src/iceberg/catalog/rest/rest_util.cc b/src/iceberg/catalog/rest/rest_util.cc index 15f093d32..cc69abe64 100644 --- a/src/iceberg/catalog/rest/rest_util.cc +++ b/src/iceberg/catalog/rest/rest_util.cc @@ -30,6 +30,15 @@ namespace iceberg::rest { +namespace { + +Status ValidateNamespaceSeparator(std::string_view separator) { + ICEBERG_PRECHECK(!separator.empty(), "REST namespace separator cannot be empty"); + return {}; +} + +} // namespace + std::string_view TrimTrailingSlash(std::string_view str) { while (!str.empty() && str.back() == '/') { str.remove_suffix(1); @@ -67,6 +76,8 @@ Result DecodeString(std::string_view str_to_decode) { Result EncodeNamespace(const Namespace& ns_to_encode, std::string_view separator) { + ICEBERG_RETURN_UNEXPECTED(ValidateNamespaceSeparator(separator)); + if (ns_to_encode.levels.empty()) { return ""; } @@ -85,6 +96,8 @@ Result EncodeNamespace(const Namespace& ns_to_encode, Result DecodeNamespace(std::string_view str_to_decode, std::string_view separator) { + ICEBERG_RETURN_UNEXPECTED(ValidateNamespaceSeparator(separator)); + if (str_to_decode.empty()) { return Namespace{.levels = {}}; } diff --git a/src/iceberg/test/rest_util_test.cc b/src/iceberg/test/rest_util_test.cc index 47431ccd3..0035afca0 100644 --- a/src/iceberg/test/rest_util_test.cc +++ b/src/iceberg/test/rest_util_test.cc @@ -25,6 +25,7 @@ #include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/endpoint.h" +#include "iceberg/catalog/rest/resource_paths.h" #include "iceberg/table_identifier.h" #include "iceberg/test/matchers.h" @@ -83,6 +84,25 @@ TEST(RestUtilTest, RoundTripNamespaceWithCustomSeparator) { HasValue(::testing::Eq(ns))); } +TEST(RestUtilTest, NamespaceRejectsEmptySeparator) { + auto expect_empty_separator_error = [](const auto& result) { + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, HasErrorMessage("REST namespace separator cannot be empty")); + }; + + expect_empty_separator_error(EncodeNamespace(Namespace{.levels = {"dogs"}}, "")); + expect_empty_separator_error(EncodeNamespace(Namespace{.levels = {}}, "")); + expect_empty_separator_error(DecodeNamespace("dogs", "")); + expect_empty_separator_error(DecodeNamespace("", "")); +} + +TEST(RestUtilTest, ResourcePathsRejectsEmptyNamespaceSeparator) { + auto result = ResourcePaths::Make("https://catalog.example.com", "", ""); + + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, HasErrorMessage("REST namespace separator cannot be empty")); +} + TEST(RestUtilTest, EncodeString) { // RFC 3986 unreserved characters should not be encoded EXPECT_THAT(EncodeString("abc123XYZ"), HasValue(::testing::Eq("abc123XYZ")));