open-telemetry / open-telemetry/opentelemetry-cpp
[BUG] ETW Properties::to_vector doubles the result and reads past a string_view
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 1.4k
- Forks
- 632
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 75
Description
Properties::to_vector(span<string_view>) in exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h:94 has two defects on adjacent lines:
std::vector<std::string> static to_vector(const nostd::span<const nostd::string_view> &source)
{
std::vector<std::string> result(source.size());
for (const auto &item : source)
{
result.push_back(std::string(item.data()));
}
return result;
}
The result is twice the size it should be. std::vector<std::string> result(source.size()) value-initialises source.size() empty strings, and the loop then appends source.size() more. A span of three views comes back as six entries whose first three are empty.
std::string(item.data()) reads until a NUL. The function's own comment says the input is a span of "non-owning string views", and nostd::string_view::data() is not required to point at a NUL terminated buffer, so this can read past the view and copy whatever follows it.
The generic overload immediately above, at line 84, is correct and does neither:
template <typename T>
static std::vector<T> to_vector(const nostd::span<const T, nostd::dynamic_extent> &source)
{
return std::vector<T>(source.begin(), source.end());
}
so the string_view overload looks like a copy that diverged.
It is live code. PropertyVariant::operator= calls it at line 260 for AttributeType::kTypeSpanString, so any ETW span or log record carrying a string-array attribute goes through it.
Why this is a report rather than a pull request
No CI job builds the ETW exporter. WITH_ETW appears in no workflow and in no branch of ci/do_ci.sh; the only workflow that names ETW at all is cppcheck.yml, which analyses rather than compiles. A fix here cannot be verified by the pipeline, and the to_vector change is entangled with the sizing bug rather than being a one line correction, so I would rather not push a blind change into a Windows-only path.
The suggested shape, if a maintainer wants it:
std::vector<std::string> result;
result.reserve(source.size());
for (const auto &item : source)
{
result.push_back(static_cast<std::string>(item));
}
return result;
nostd::string_view has an explicit operator std::string() that copies by length.
I am happy to send that, but it would be more useful alongside a job that compiles the ETW exporter, since the sizing bug is the kind a single assertion would have caught. Found while sweeping the same string_view::data() shape for #4346, which fixes the four occurrences in components that CI does build.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h around the generic overload at line 84 and the string_view overload at line 94, then inspect PropertyVariant::operator= around line 260. Verify the ETW exporter on a span of string views, ensuring the returned vector has the same length and preserves non-NUL-terminated view contents; note that no CI job currently builds ETW.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- observability-sre
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100