Bug Fix: Proper Handle Zero-Length Binary Data

Add code to proper handle the case of zero-length binary data in the registry. Also improved existing code using std::vector::data() instead of &v[0] to access vector's elements as a C-style raw array pointer (used e.g. for passing array data to/from Win32 APIs).
GiovanniDicanio · Jul 1, 2022 · d44f07f · d44f07f
1 parent 4ac74bf
commit d44f07f
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 23 deletions.
diff --git a/README.md b/README.md
@@ -1,4 +1,4 @@
-# WinReg v5.1.0
+# WinReg v5.1.1
 ## High-level C++ Wrapper Around the Low-level Windows Registry C-interface API
 
 by Giovanni Dicanio

diff --git a/WinReg/WinReg.hpp b/WinReg/WinReg.hpp
@@ -9,7 +9,7 @@
 //               Copyright (C) by Giovanni Dicanio
 //
 // First version: 2017, January 22nd
-// Last update:   2022, March 29th
+// Last update:   2022, July 1st
 //
 // E-mail: <first name>.<last name> AT REMOVE_THIS gmail.com
 //
@@ -773,7 +773,7 @@ class ScopedLocalFree
     // -------------------------------------------------------------------------
     //// *** Previous parsing code - Assumes an empty string terminates the sequence ***
     //
-    //const wchar_t* currStringPtr = &data[0];
+    //const wchar_t* currStringPtr = data.data();
     //while (*currStringPtr != L'\0')
     //{
     //    // Current string is NUL-terminated, so get its length calling wcslen
@@ -788,8 +788,8 @@ class ScopedLocalFree
     // -------------------------------------------------------------------------
     //
 
-    const wchar_t* currStringPtr = &data[0];
-    const wchar_t* const endPtr  = &data[0] + data.size() - 1;
+    const wchar_t* currStringPtr = data.data();
+    const wchar_t* const endPtr  = data.data() + data.size() - 1;
 
     while (currStringPtr < endPtr)
     {
@@ -1226,7 +1226,7 @@ inline void RegKey::SetMultiStringValue(
         valueName.c_str(),
         0, // reserved
         REG_MULTI_SZ,
-        reinterpret_cast<const BYTE*>(&multiString[0]),
+        reinterpret_cast<const BYTE*>(multiString.data()),
         dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1248,7 +1248,7 @@ inline void RegKey::SetBinaryValue(const std::wstring& valueName, const std::vec
         valueName.c_str(),
         0, // reserved
         REG_BINARY,
-        &data[0],
+        data.data(),
         dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1371,7 +1371,7 @@ inline RegResult RegKey::TrySetMultiStringValue(const std::wstring& valueName,
         valueName.c_str(),
         0, // reserved
         REG_MULTI_SZ,
-        reinterpret_cast<const BYTE*>(&multiString[0]),
+        reinterpret_cast<const BYTE*>(multiString.data()),
         dataSize
     ) };
 }
@@ -1390,7 +1390,7 @@ inline RegResult RegKey::TrySetBinaryValue(const std::wstring& valueName,
         valueName.c_str(),
         0, // reserved
         REG_BINARY,
-        &data[0],
+        data.data(),
         dataSize
     ) };
 }
@@ -1497,8 +1497,8 @@ inline std::wstring RegKey::GetStringValue(const std::wstring& valueName) const
         nullptr,    // no subkey
         valueName.c_str(),
         flags,
-        nullptr,    // type not required
-        &result[0], // output buffer
+        nullptr,       // type not required
+        result.data(), // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1555,8 +1555,8 @@ inline std::wstring RegKey::GetExpandStringValue(
         nullptr,    // no subkey
         valueName.c_str(),
         flags,
-        nullptr,    // type not required
-        &result[0], // output buffer
+        nullptr,       // type not required
+        result.data(), // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1605,7 +1605,7 @@ inline std::vector<std::wstring> RegKey::GetMultiStringValue(const std::wstring&
         valueName.c_str(),
         flags,
         nullptr,    // no type required
-        &data[0],   // output buffer
+        data.data(),   // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1648,14 +1648,22 @@ inline std::vector<BYTE> RegKey::GetBinaryValue(const std::wstring& valueName) c
     // Allocate a buffer of proper size to store the binary data
     std::vector<BYTE> data(dataSize);
 
+    // Handle the special case of zero-length binary data:
+    // If the binary data value in the registry is empty, just return
+    if (dataSize == 0)
+    {
+        _ASSERTE(data.empty());
+        return data;
+    }
+
     // Call RegGetValue for the second time to read the data content
     retCode = ::RegGetValueW(
         m_hKey,
         nullptr,    // no subkey
         valueName.c_str(),
         flags,
         nullptr,    // type not required
-        &data[0],   // output buffer
+        data.data(),   // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1748,11 +1756,11 @@ inline std::optional<std::wstring> RegKey::TryGetStringValue(const std::wstring&
     // Call RegGetValue for the second time to read the string's content
     retCode = ::RegGetValueW(
         m_hKey,
-        nullptr,    // no subkey
+        nullptr,        // no subkey
         valueName.c_str(),
         flags,
-        nullptr,    // type not required
-        &result[0], // output buffer
+        nullptr,        // type not required
+        result.data(),  // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1806,11 +1814,11 @@ inline std::optional<std::wstring> RegKey::TryGetExpandStringValue(
     // Call RegGetValue for the second time to read the string's content
     retCode = ::RegGetValueW(
         m_hKey,
-        nullptr,    // no subkey
+        nullptr,        // no subkey
         valueName.c_str(),
         flags,
-        nullptr,    // type not required
-        &result[0], // output buffer
+        nullptr,        // type not required
+        result.data(),  // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1859,7 +1867,7 @@ inline std::optional<std::vector<std::wstring>>
         valueName.c_str(),
         flags,
         nullptr,    // no type required
-        &data[0],   // output buffer
+        data.data(),   // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)
@@ -1903,14 +1911,22 @@ inline std::optional<std::vector<BYTE>>
     // Allocate a buffer of proper size to store the binary data
     std::vector<BYTE> data(dataSize);
 
+    // Handle the special case of zero-length binary data:
+    // If the binary data value in the registry is empty, just return
+    if (dataSize == 0)
+    {
+        _ASSERTE(data.empty());
+        return data;
+    }
+
     // Call RegGetValue for the second time to read the data content
     retCode = ::RegGetValueW(
         m_hKey,
         nullptr,    // no subkey
         valueName.c_str(),
         flags,
         nullptr,    // type not required
-        &data[0],   // output buffer
+        data.data(),   // output buffer
         &dataSize
     );
     if (retCode != ERROR_SUCCESS)

diff --git a/WinReg/WinRegTest.cpp b/WinReg/WinRegTest.cpp
@@ -73,6 +73,7 @@ void Test()
     const wstring testSz = L"CiaoTestSz";
     const wstring testExpandSz = L"%PATH%";
     const vector<BYTE> testBinary = { 0xAA, 0xBB, 0xCC, 0x11, 0x22, 0x33 };
+    const vector<BYTE> testEmptyBinary; // used to test zero-length binary data array
     const vector<wstring> testMultiSz = { L"Hi", L"Hello", L"Ciao" };
 
     key.SetDwordValue(L"TestValueDword", testDw);
@@ -81,6 +82,8 @@ void Test()
     key.SetExpandStringValue(L"TestValueExpandString", testExpandSz);
     key.SetMultiStringValue(L"TestValueMultiString", testMultiSz);
     key.SetBinaryValue(L"TestValueBinary", testBinary);
+    key.SetBinaryValue(L"TestEmptyBinary", testEmptyBinary);
+    // TODO: May add tests for other empty values, like empty string, etc.
 
     if (key.TrySetDwordValue(L"TestTryValueDword", testDw).Failed())
     {
@@ -112,6 +115,11 @@ void Test()
         wcout << L"RegKey::TrySetBinaryValue failed.\n";
     }
 
+    if (key.TrySetBinaryValue(L"TestTryEmptyBinary", testEmptyBinary).Failed())
+    {
+        wcout << L"RegKey::TrySetBinaryValue failed with zero-length binary array.\n";
+    }
+
 
     DWORD testDw1 = key.GetDwordValue(L"TestValueDword");
     if (testDw1 != testDw)
@@ -257,6 +265,25 @@ void Test()
         wcout << L"RegKey::QueryValueType failed for REG_BINARY.\n";
     }
 
+    // Test the special case of zero-length binary array
+    vector<BYTE> testEmptyBinary1 = key.GetBinaryValue(L"TestEmptyBinary");
+    if (testEmptyBinary1 != testEmptyBinary)
+    {
+        wcout << L"RegKey::GetBinaryValue failed with zero-length binary data.\n";
+    }
+
+    if (auto testEmptyBinary2 = key.TryGetBinaryValue(L"TestTryEmptyBinary"))
+    {
+        if (testEmptyBinary2 != testEmptyBinary)
+        {
+            wcout << L"RegKey::TryGetBinaryValue failed with zero-length binary data.\n";
+        }
+    }
+    else
+    {
+        wcout << L"RegKey::TryGetBinaryValue failed (std::optional has no value).\n";
+    }
+
 
     //
     // Remove some test values