Restrict TLVRecord getValueAs/setValue to trivially copyable types

[alacrity-aya]

Bug description

Describe the bug

TLVRecord::getValueAs<T>() and TLVRecord::setValue<T>() currently accept arbitrary types, although both functions use memcpy to copy raw bytes to and from objects of type T.

This implementation is only safe for types whose object representation can be copied byte-for-byte, such as trivially copyable types. However, the current API does not enforce this requirement at compile time.

For example, the following code compiles successfully:

auto value = dhcpLayer
                ->getOptionData(pcpp::DHCPOPT_DHCP_LEASE_TIME)
                .getValueAs<std::string>();

std::string is not a trivially copyable type, so using it with getValueAs<T>() results in invalid behavior.

1. Insufficient-data path

When the record data is smaller than sizeof(T) + offset, getValueAs<T>() currently returns:

return 0;

For T = std::string, the integer literal 0 can be converted to a null pointer and then used to initialize a std::string through its const char* constructor.

For example:

std::string str(nullptr);

With GCC's libstdc++, this results in a runtime exception:

basic_string: construction from null is not valid

Changing the return expression to:

return static_cast<T>(0);

does not solve the problem, because static_cast<std::string>(0) is still accepted.

The insufficient-data path should instead use value-initialization:

return T{};

2. Sufficient-data path

When the record contains enough data, getValueAs<T>() performs:

memcpy(&result, ..., sizeof(T));

For non-trivially copyable types, this overwrites the internal object representation directly, bypassing constructors and object invariants. For types such as std::string, this can corrupt internal state and results in undefined behavior.

setValue<T>() has the same underlying issue because it also performs a raw memcpy without constraining T.

C++ version note

This invalid usage compiles without a compile-time error in C++11, C++14, C++17, and C++20.

The problem is therefore not specific to a particular C++ standard mode: the API currently allows types that are incompatible with its memcpy-based implementation.

Code example to reproduce

// getValueAs<T>() currently accepts a non-trivially copyable type.
// This compiles, but the implementation is not valid for std::string.
auto value = dhcpLayer
                ->getOptionData(pcpp::DHCPOPT_DHCP_LEASE_TIME)
                .getValueAs<std::string>();

Minimal reproduction of the runtime failure

The following reproduces the behavior of the insufficient-data path for T = std::string:

#include <iostream>
#include <string>

int main()
{
    std::cout << "sizeof(std::string) = " << sizeof(std::string) << "\n";
    std::string str(0);
    return 0;
}

Example compilation and output:

❯ clang++ main.cc -std=c++14 -Wall -Wextra -O0
main.cc:4:14: warning: unused parameter 'argc' [-Wunused-parameter]
    4 | int main(int argc, char* argv[]) {
      |              ^
main.cc:4:26: warning: unused parameter 'argv' [-Wunused-parameter]
    4 | int main(int argc, char* argv[]) {
      |                          ^
2 warnings generated.
❯ ./a.out
sizeof(std::string) = 32
terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string: construction from null is not valid
[1]    87732 IOT instruction (core dumped)  ./a.out

Expected behavior

getValueAs<T>() and setValue<T>() should reject unsupported types at compile time because their implementations rely on raw memcpy of object representations.

In particular:

• Non-trivially copyable types such as std::string should fail at compile time with a clear diagnostic near the call site.
• getValueAs<T>() should use value-initialization for the insufficient-data path:

return T{};

PcapPlusPlus versions tested on

PcapPlusPlus master branch

Other PcapPlusPlus version (if applicable)

No response

Operating systems tested on

Linux

Other operation systems (if applicable)

No response

Compiler version

clang version 22.1.8 (Fedora 22.1.8-4.fc44)

Packet capture backend (if applicable)

No response

[Dimi1010]

For non-tivial types. I suppose a SFINAE std::enable_if pattern on the template would be a solution. Not sure if I would call their compile failure diagnostics "clear" but it is better than nothing.

[alacrity-aya]

For non-tivial types. I suppose a SFINAE std::enable_if pattern on the template would be a solution. Not sure if I would call their compile failure diagnostics "clear" but it is better than nothing.

For non-trivial types, static_assert may be a better fit 🤔, for example:

template <typename T> T getValueAs(size_t offset = 0) const
{
	static_assert(std::is_trivially_copyable<T>::value && !std::is_pointer<T>::value,
			        "TLVRecord::getValueAs<T>() requires T to be a non-pointer, trivially copyable type");

	if (getDataSize() < sizeof(T) + offset)
		return T{};

	T result{};
	memcpy(&result, m_Data->recordValue + getValueOffset() + offset, sizeof(T));
	return result;
}

In this case, we are not trying to make the function participate differently based on T; we simply want to reject unsupported types and provide a direct diagnostic when the template is instantiated.