Skip to content

fix: Store fixed-size strings inline in dd_core_config_t#320

Open
awforsythe wants to merge 1 commit into
developfrom
aforsythe/RUM-17284/c-api-string-safety
Open

fix: Store fixed-size strings inline in dd_core_config_t#320
awforsythe wants to merge 1 commit into
developfrom
aforsythe/RUM-17284/c-api-string-safety

Conversation

@awforsythe

Copy link
Copy Markdown
Contributor

refs: RUM-17284

Background

In the C++ API, where we accept values like env, version, service, etc. as std::string_view, we implicitly copy the application-provided values into std::string members.

In the C API, we need to hold string values in dd_core_config_t initially: when the core is created via dd_core_create(), we convert all of our config data to a C++ CoreConfig struct

Problem

The C API holds some string values as char* - we accept raw string pointers from the application and store them directly. This is fine for static strings, but it represents a use-after-free bug if the application-provided values have limited lifetime: we could crash on dd_core_create() if values supplied for client_token, env, version, service, etc., have temporary storage.

Fix

This PR fixes the problem by making dd_core_config_t (and its member struct dd_internal_options_t) store all string values inline, using fixed-size buffers. These fixed sizes are exposed as public constants: we already took this approach to prevent similar issues in the C logger API.

If an application-provided string exceeds the expected maximum size, we truncate it and log a warning.

This is a breaking ABI change (and it also renames a public constant in the C API, which technically makes it a breaking API change as well) that only affects the C API. Given the early stage of C API adoption, I think we should be fine to make this change swiftly.

After the initial Flutter desktop pass came together, we briefly discussed making more fundamental changes to the way that config objects are allocated and stored in the C API. These changes are not in scope here: this PR is concerned only with preventing this specific class of bugs.

refs: RUM-17284

 ## Background

In the C++ API, where we accept values like `env`, `version`, `service`,
etc. as `std::string_view`, we implicitly copy the application-provided
values into `std::string` members.

In the C API, we need to hold string values in `dd_core_config_t`
initially: when the core is created via `dd_core_create()`, we convert
all of our config data to a C++ `CoreConfig` struct

 ## Problem

The C API holds some string values as `char*` - we accept raw string
pointers from the application and store them directly. This is fine for
static strings, but it represents a use-after-free bug if the
application-provided values have limited lifetime: we could crash on
`dd_core_create()` if values supplied for client_token, env, version,
service, etc., have temporary storage.

 ## Fix

This PR fixes the problem by making `dd_core_config_t` (and its member
struct `dd_internal_options_t`) store all string values inline, using
fixed-size buffers. These fixed sizes are exposed as public constants:
we already took this approach to prevent similar issues in the C logger
API.

If an application-provided string exceeds the expected maximum size, we
truncate it and log a warning.

This is a breaking ABI change (and it also renames a public constant in
the C API, which technically makes it a breaking API change as well)
that only affects the C API. Given the early stage of C API adoption, I
think we should be fine to make this change swiftly.

After the initial Flutter desktop pass came together, we briefly
discussed making more fundamental changes to the way that config objects
are allocated and stored in the C API. These changes are not in scope
here: this PR is concerned only with preventing this specific class of
bugs.
@awforsythe

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: 6fe4b539c7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@awforsythe awforsythe marked this pull request as ready for review July 10, 2026 18:03
@awforsythe awforsythe requested review from a team as code owners July 10, 2026 18:03
@sbarrio sbarrio requested a review from arroz July 13, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant