Fix handling of empty strings on windows

ph3at · Feb 7, 2024 · 6547988 · 6547988
1 parent f1076eb
commit 6547988
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 5 deletions.
diff --git a/source/libenvpp_environment_windows.cpp b/source/libenvpp_environment_windows.cpp
@@ -14,6 +14,9 @@ namespace env::detail {
 
 std::optional<std::string> convert_string(const std::wstring& str)
 {
+	if (str.empty()) {
+		return "";
+	}
 	const auto buffer_size =
 	    WideCharToMultiByte(CP_UTF8, 0, str.c_str(), static_cast<int>(str.length()), nullptr, 0, nullptr, nullptr);
 	if (buffer_size == 0) {
@@ -28,6 +31,9 @@ std::optional<std::string> convert_string(const std::wstring& str)
 
 std::optional<std::wstring> convert_string(const std::string& str)
 {
+	if (str.empty()) {
+		return L"";
+	}
 	const auto buffer_size = MultiByteToWideChar(CP_UTF8, 0, str.c_str(), static_cast<int>(str.length()), nullptr, 0);
 	if (buffer_size == 0) {
 		return {};
@@ -61,7 +67,7 @@ std::optional<std::wstring> convert_string(const std::string& str)
 		if (!var_name_value[0].empty()) {
 			auto key = convert_string(var_name_value[0]);
 			auto value = convert_string(var_name_value[1]);
-			if(key && value) {
+			if (key && value) {
 				env_map[*key] = *value;
 			}
 		}
@@ -76,31 +82,38 @@ std::optional<std::wstring> convert_string(const std::string& str)
 [[nodiscard]] std::optional<std::string> get_environment_variable(const std::string_view name)
 {
 	const auto var_name = convert_string(std::string(name));
-	if(!var_name) return {};
+	if (!var_name) {
+		return {};
+	}
 	const auto buffer_size = GetEnvironmentVariableW(var_name->c_str(), nullptr, 0);
 	if (buffer_size == 0) {
 		return {};
 	}
 	// -1 because std::string already contains implicit null terminator
 	auto value = std::wstring(buffer_size - 1, L'\0');
 	[[maybe_unused]] const auto env_var_got = GetEnvironmentVariableW(var_name->c_str(), value.data(), buffer_size);
-	LIBENVPP_CHECK(env_var_got != 0);
+	// An empty string will have buffer_size == 1 and thus read 0, which is not an error.
+	LIBENVPP_CHECK(env_var_got != 0 || buffer_size == 1);
 	return convert_string(value);
 }
 
 void set_environment_variable(const std::string_view name, const std::string_view value)
 {
 	auto key = convert_string(std::string(name));
 	auto val = convert_string(std::string(value));
-	if(!key || !val) throw std::runtime_error("libenvpp: set_environment_variable failed in string conversion");
+	if (!key || !val) {
+		throw std::runtime_error("libenvpp: set_environment_variable failed in string conversion");
+	}
 	[[maybe_unused]] const auto env_var_was_set = SetEnvironmentVariableW(key->c_str(), val->c_str());
 	LIBENVPP_CHECK(env_var_was_set);
 }
 
 void delete_environment_variable(const std::string_view name)
 {
 	auto key = convert_string(std::string(name));
-	if(!key) throw std::runtime_error("libenvpp: delete_environment_variable failed in string conversion");
+	if (!key) {
+		throw std::runtime_error("libenvpp: delete_environment_variable failed in string conversion");
+	}
 	[[maybe_unused]] const auto env_var_was_deleted = SetEnvironmentVariableW(key->c_str(), nullptr);
 	LIBENVPP_CHECK(env_var_was_deleted);
 }

diff --git a/test/libenvpp_environment_test.cpp b/test/libenvpp_environment_test.cpp
@@ -23,6 +23,17 @@ TEST_CASE("Converting wide-char to multi-byte", "[libenvpp_env]")
 		CHECK_THAT(*multi_byte_str, Equals(output));
 	}
 
+	SECTION("Empty input")
+	{
+		constexpr auto input = L"";
+		constexpr auto output = "";
+
+		const auto wide_char_str = std::wstring(input);
+		const auto multi_byte_str = convert_string(wide_char_str);
+		REQUIRE(multi_byte_str.has_value());
+		CHECK_THAT(*multi_byte_str, Equals(output));
+	}
+
 	SECTION("Invalid input parsed at least up to invalid character")
 	{
 		constexpr auto input = L"foo \xd800 bar";
@@ -58,6 +69,17 @@ TEST_CASE("Converting multi-byte to wide-char", "[libenvpp_env]")
 		CHECK(*wide_char_str == output); // Catch2 does not have matcher support for wide-chars
 	}
 
+	SECTION("Empty input")
+	{
+		constexpr auto input = "";
+		constexpr auto output = L"";
+
+		const auto multi_byte_str = std::string(input);
+		const auto wide_char_str = convert_string(multi_byte_str);
+		REQUIRE(wide_char_str.has_value());
+		CHECK(*wide_char_str == output); // Catch2 does not have matcher support for wide-chars
+	}
+
 	SECTION("Invalid input parsed at least up to invalid character")
 	{
 		constexpr auto input = "foo \x80 bar";
@@ -241,6 +263,24 @@ TEST_CASE("Character encoding for variable values", "[libenvpp_env]")
 		CHECK_THAT(environment.at(test_var_name), Equals(test_var_value));
 	}
 
+	SECTION("Empty input")
+	{
+		constexpr auto test_var_name = "LIBENVPP_TESTING_BANANA";
+		constexpr auto test_var_value = "";
+
+		const auto _ = set_scoped_environment_variable{test_var_name, test_var_value};
+
+		const auto test_var = get_environment_variable(test_var_name);
+		REQUIRE(test_var.has_value());
+		CHECK_THAT(*test_var, Equals(test_var_value));
+
+		const auto environment = get_environment();
+		REQUIRE_FALSE(environment.empty());
+
+		REQUIRE(environment.find(test_var_name) != environment.end());
+		CHECK_THAT(environment.at(test_var_name), Equals(test_var_value));
+	}
+
 	SECTION("Invalid input")
 	{
 		constexpr auto test_var_name = "LIBENVPP_TESTING_INVALID_UTF-8";