Skip to content

descriptor: validate reflective field membership#254

Open
fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:fix/validate-reflect-field-membership
Open

descriptor: validate reflective field membership#254
fallintoplace wants to merge 1 commit into
anthropics:mainfrom
fallintoplace:fix/validate-reflect-field-membership

Conversation

@fallintoplace

Copy link
Copy Markdown
Contributor

Summary

Add checked reflective mutation APIs that validate field-descriptor membership before mutating a DynamicMessage.

Why

ReflectMessageMut::set and clear were accepting any FieldDescriptor and indexing directly by field number. If a caller passed a descriptor from another message or another pool with the same number, the dynamic message could silently mutate the wrong field in release builds.

What changed

  • add ReflectError plus checked try_set and try_clear on ReflectMessageMut
  • validate membership in DynamicMessage before mutating state
  • make the legacy set and clear paths panic on foreign descriptors instead of silently mutating a colliding field number
  • add debug membership assertions to has for parity with get and field_mut
  • add regression coverage for same-pool and cross-pool foreign descriptors, plus the panic compatibility path

Compatibility

I kept the existing set and clear signatures intact to avoid a trait break. Callers that want recoverable errors can switch to try_set and try_clear.

Validation

  • cargo test -p buffa-descriptor --features reflect

@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

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