fix crash when passing a std::string containing null terminated chara… (

#177) * fix crash when passing a std::string containing null terminated characters inside fixes #176
odygrd · May 30, 2022 · b259b7f · b259b7f
1 parent 51293a3
commit b259b7f
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,4 @@
+- [v2.0.2](#v2.0.2)
 - [v2.0.1](#v2.0.1)
 - [v2.0.0](#v2.0.0)
 - [v1.7.3](#v1.7.3)
@@ -24,6 +25,12 @@
 - [v1.1.0](#v1.1.0)
 - [v1.0.0](#v1.0.0)
 
+## v2.0.2
+
+**Fixes**
+
+- Fix crash when a `std::string` containing null-terminated characters is passed to the logger. ([#176](https://github.com/odygrd/quill/issues/176))
+
 ## v2.0.1
 
 **Improvements**

diff --git a/quill/include/quill/Quill.h b/quill/include/quill/Quill.h
@@ -27,7 +27,7 @@ namespace quill
 /** Version Info **/
 constexpr uint32_t VersionMajor{2};
 constexpr uint32_t VersionMinor{0};
-constexpr uint32_t VersionPatch{1};
+constexpr uint32_t VersionPatch{2};
 constexpr uint32_t Version{VersionMajor * 10000 + VersionMinor * 100 + VersionPatch};
 
 /** forward declarations **/

diff --git a/quill/include/quill/detail/Serialize.h b/quill/include/quill/detail/Serialize.h
@@ -89,20 +89,40 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT inline std::byte* decode_args(
 {
   using ArgType = detail::remove_cvref_t<Arg>;
 
-  if constexpr (is_type_of_c_string<Arg>() || is_type_of_string<Arg>())
+  if constexpr (is_type_of_c_string<Arg>())
   {
     char const* str = reinterpret_cast<char const*>(in);
     std::string_view const v{str, strlen(str)};
     args.emplace_back(fmt::detail::make_arg<fmt::format_context>(v));
     return decode_args<DestructIdx, Args...>(in + v.length() + 1, args, destruct_args);
   }
+  else if constexpr (is_type_of_string<Arg>())
+  {
+    // for std::string we first need to retrieve the length
+    in = detail::align_pointer<alignof(Arg), std::byte>(in);
+    size_t len{0};
+    std::memcpy(&len, in, sizeof(size_t));
+    in += sizeof(size_t);
+
+    // retrieve the rest of the string
+    char const* str = reinterpret_cast<char const*>(in);
+    std::string_view const v{str, len};
+    args.emplace_back(fmt::detail::make_arg<fmt::format_context>(v));
+    return decode_args<DestructIdx, Args...>(in + v.length(), args, destruct_args);
+  }
 #if defined(_WIN32)
   else if constexpr (is_type_of_wide_c_string<Arg>() || is_type_of_wide_string<Arg>())
   {
+    // for std::wstring we first need to retrieve the length
+    in = detail::align_pointer<alignof(Arg), std::byte>(in);
+    size_t len{0};
+    std::memcpy(&len, in, sizeof(size_t));
+    in += sizeof(size_t);
+
     char const* str = reinterpret_cast<char const*>(in);
-    std::string_view const v{str, strlen(str)};
+    std::string_view const v{str, len};
     args.emplace_back(fmt::detail::make_arg<fmt::format_context>(v));
-    return decode_args<DestructIdx, Args...>(in + v.length() + 1, args, destruct_args);
+    return decode_args<DestructIdx, Args...>(in + v.length(), args, destruct_args);
   }
 #endif
   else
@@ -165,20 +185,25 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT constexpr size_t get_args_sizes(size_t* c_st
   }
   else if constexpr (is_type_of_string<Arg>())
   {
-    return (arg.size() + 1) + get_args_sizes<CstringIdx>(c_string_sizes, args...);
+    // for std::string we also need to store the size in order to correctly retrieve it
+    // the reason for this is that if we create e.g:
+    // std::string msg = fmt::format("{} {} {} {} {}", (char)0, (char)0, (char)0, (char)0,
+    // "sssssssssssssssssssssss"); then strlen(msg.data()) = 0 but msg.size() = 31
+    return (arg.size() + sizeof(size_t)) + alignof(size_t) +
+      get_args_sizes<CstringIdx>(c_string_sizes, args...);
   }
 #if defined(_WIN32)
   else if constexpr (is_type_of_wide_c_string<Arg>())
   {
-    size_t const len = get_wide_string_encoding_size(std::wstring_view {arg, wcslen(arg)}) + 1;
+    size_t const len = get_wide_string_encoding_size(std::wstring_view {arg, wcslen(arg)});
     c_string_sizes[CstringIdx] = len;
-    return len + get_args_sizes<CstringIdx + 1>(c_string_sizes, args...);
+    return len + sizeof(size_t) + alignof(size_t) + get_args_sizes<CstringIdx + 1>(c_string_sizes, args...);
   }
   else if constexpr (is_type_of_wide_string<Arg>())
   {
-    size_t const len = get_wide_string_encoding_size(arg) + 1;
+    size_t const len = get_wide_string_encoding_size(arg);
     c_string_sizes[CstringIdx] = len;
-    return len + get_args_sizes<CstringIdx + 1>(c_string_sizes, args...);
+    return len + sizeof(size_t) + alignof(size_t) + get_args_sizes<CstringIdx + 1>(c_string_sizes, args...);
   }
 #endif
   else
@@ -208,19 +233,36 @@ QUILL_NODISCARD QUILL_ATTRIBUTE_HOT constexpr std::byte* encode_args(size_t* c_s
   }
   else if constexpr (is_type_of_string<Arg>())
   {
+    // for std::string we store the size first, in order to correctly retrieve it
+    out = detail::align_pointer<alignof(Arg), std::byte>(out);
+    size_t const len = arg.length();
+    std::memcpy(out, &len, sizeof(size_t));
+    out += sizeof(size_t);
+
+    // copy the string, no need to zero terminate it as we got the length
     std::memcpy(out, arg.data(), arg.length());
-    out[arg.length()] = static_cast<std::byte>(0);
-    return encode_args<CstringIdx>(c_string_sizes, out + arg.length() + 1, std::forward<Args>(args)...);
+    return encode_args<CstringIdx>(c_string_sizes, out + arg.length(), std::forward<Args>(args)...);
   }
 #if defined(_WIN32)
   else if constexpr (is_type_of_wide_c_string<Arg>())
   {
+    out = detail::align_pointer<alignof(Arg), std::byte>(out);
+    size_t const len = c_string_sizes[CstringIdx];
+    std::memcpy(out, &len, sizeof(size_t));
+
+    out += sizeof(size_t);
     wide_string_to_narrow(out, c_string_sizes[CstringIdx], std::wstring_view{arg, wcslen(arg)});
     return encode_args<CstringIdx + 1>(c_string_sizes, out + c_string_sizes[CstringIdx],
                                        std::forward<Args>(args)...);
   }
   else if constexpr (is_type_of_wide_string<Arg>())
   {
+    // for std::wstring we store the size first, in order to correctly retrieve it
+    out = detail::align_pointer<alignof(Arg), std::byte>(out);
+    size_t const len = c_string_sizes[CstringIdx];
+    std::memcpy(out, &len, sizeof(size_t));
+    out += sizeof(size_t);
+
     wide_string_to_narrow(out, c_string_sizes[CstringIdx], arg);
     return encode_args<CstringIdx + 1>(c_string_sizes, out + c_string_sizes[CstringIdx],
                                        std::forward<Args>(args)...);