1
0
mirror of https://github.com/esphome/esphome.git synced 2025-10-22 11:43:51 +01:00

[api_protobuf] Address copilot review: add bounds checking and clarify 32-bit loop intent

- Add bounds checking in decode_varint_content to prevent undefined behavior if decoded enum value exceeds 31
- Add clarifying comments that 32-bit loops in encode_content and get_size_calculation are intentional to support the full range of enum_as_bitmask (enums with up to 32 values)
- The uint32_t storage type supports general-purpose enum_as_bitmask, not just ColorMode's 10 values
This commit is contained in:
J. Nick Koston
2025-10-18 11:56:40 -10:00
parent 27b876df93
commit ef52ce4d76

View File

@@ -1472,11 +1472,16 @@ class RepeatedTypeInfo(TypeInfo):
if self._use_pointer: if self._use_pointer:
return None return None
if self._use_bitmask: if self._use_bitmask:
# For bitmask fields, decode enum value and set corresponding bit # For bitmask fields, decode enum value and set corresponding bit, with bounds checking
content = self._ti.decode_varint content = self._ti.decode_varint
if content is None: if content is None:
return None return None
return f"case {self.number}: this->{self.field_name} |= (1U << static_cast<uint8_t>({content})); break;" return (
f"case {self.number}: "
f"if (static_cast<uint8_t>({content}) <= 31) "
f"this->{self.field_name} |= (1U << static_cast<uint8_t>({content})); "
f"break;"
)
content = self._ti.decode_varint content = self._ti.decode_varint
if content is None: if content is None:
return None return None
@@ -1533,6 +1538,9 @@ class RepeatedTypeInfo(TypeInfo):
if self._use_bitmask: if self._use_bitmask:
# For bitmask fields, iterate through set bits and encode each enum value # For bitmask fields, iterate through set bits and encode each enum value
# The bitmask is stored as uint32_t where bit N represents enum value N # The bitmask is stored as uint32_t where bit N represents enum value N
# Note: We iterate through all 32 bits to support the full range of enum_as_bitmask
# (enums with up to 32 values). Specific uses may have fewer values, but the
# generated code is general-purpose.
assert isinstance(self._ti, EnumType), ( assert isinstance(self._ti, EnumType), (
"enum_as_bitmask only works with enum fields" "enum_as_bitmask only works with enum fields"
) )
@@ -1587,6 +1595,9 @@ class RepeatedTypeInfo(TypeInfo):
if self._use_bitmask: if self._use_bitmask:
# For bitmask fields, iterate through set bits and calculate size # For bitmask fields, iterate through set bits and calculate size
# Each set bit encodes one enum value (as varint) # Each set bit encodes one enum value (as varint)
# Note: We iterate through all 32 bits to support the full range of enum_as_bitmask
# (enums with up to 32 values). Specific uses may have fewer values, but the
# generated code is general-purpose.
o = f"if ({name} != 0) {{\n" o = f"if ({name} != 0) {{\n"
o += " for (uint8_t bit = 0; bit < 32; bit++) {\n" o += " for (uint8_t bit = 0; bit < 32; bit++) {\n"
o += f" if ({name} & (1U << bit)) {{\n" o += f" if ({name} & (1U << bit)) {{\n"