Skip to content

PR #852: From<ServerDeviceFeatureOutput> for DeviceFeatureOutput loses disabled-output filtering #873

@qdot

Description

@qdot

Description

PR #852 refactors ServerDeviceFeatureOutput from a struct with Option fields to an enum stored in SmallVecEnumMap. As part of this, the From<ServerDeviceFeatureOutput> for DeviceFeatureOutput impl is replaced by a macro-generated version that no longer filters disabled outputs.

Commit bc88d3d specifically added .filter(|x| !x.disabled()) to the old From impl to prevent disabled outputs from leaking to clients. The new macro-generated From impl converts all variants unconditionally.

The filtering responsibility has moved to as_device_feature(), which correctly calls .filter(|o| !o.is_disabled()) before the conversion. This works today since as_device_feature() is the only call path. However, the public From impl is now a footgun — any future code calling .into() on a &ServerDeviceFeatureOutput directly will bypass the disabled filter.

Suggested fix

Either:

  1. Restore filtering in the From impl (or make it a named method instead of From to signal it's not a simple conversion)
  2. Make the From impl pub(crate) to limit the blast radius
  3. Document the invariant that as_device_feature() is the only safe conversion path

Context

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions