Skip to content

Fix RawPropsParser ownership of temporary property names - #59479

Closed
Shubham-Padkonde wants to merge 1 commit into
react:mainfrom
Shubham-Padkonde:fix/rawprops-owned-composed-keys
Closed

Shubham-Padkonde wants to merge 1 commit into
react:mainfrom
Shubham-Padkonde:fix/rawprops-owned-composed-keys

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown

Summary:

Fixes #59327.

RawPropsParser keeps property names after preparation, but its string views can refer to temporary names. In particular, RawProps::at(name, prefix, suffix) passes a view into a local concatenated string. Later duplicate checks and cursor lookups read that string after it has been destroyed.

Store owned strings in the parser and construct each stored key when it is first registered. Lookup still accepts string views; the copy happens during parser preparation. The key map already copies names into its own storage.

Changelog:

[GENERAL] [FIXED] - Retain property names used to prepare RawPropsParser so temporary and composed names remain valid during later lookups.

Test Plan:

Added two GoogleTest regressions covering short and long prefixes/suffixes, duplicate registration, repeated and out-of-order lookups, and temporary unprefixed names across vector growth.

Linux AddressSanitizer validation compiles the actual RawProps, RawPropsParser, RawPropsKeyMap and JSI sources with the new test file and React Native's Folly source configuration:

  • ASAN_OPTIONS=detect_stack_use_after_return=1 build-rawprops/rawprops_tests: both tests pass.
  • Restoring the two production files from base commit 54b5f22c33773a83de8fb6dc9a7091ba180842b4 and rebuilding makes each regression fail independently: stack-use-after-return for composed names and stack-use-after-scope for temporary names.
  • clang-format 21.1.2 checks the new test file successfully; git diff --check passes.

The focused harness and its workflow are on a separate fork branch, outside this PR. Full Android/iOS builds and the full native suite were not run.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 26, 2026
@javache

javache commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

RawProps.at is only supposed to be used with const char* args in constructor methods, for which I believe this is safe.

Changing string_view to string significantly changes the memory costs here, and we should avoid it.

@javache javache closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RawProps::at(name, prefix, suffix) leaves RawPropsParser a view of a destroyed key

2 participants