From 1e5f85448c2d91f608dfc72218c4f5c0b225c2a4 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Fri, 26 Dec 2025 14:47:53 -0700 Subject: [PATCH 1/6] Return nil for items outside buffer bounds in undersized packets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a packet with ALLOW_SHORT receives an undersized buffer, items that are completely outside the buffer bounds now return nil/None instead of 0. Previously, undersized buffers were padded with zeros which caused out-of-bounds items to return 0. Changes: - Structure no longer resizes buffer when short_buffer_allowed is true - BinaryAccessor.read_item checks bounds before reading and returns nil/None for items outside the buffer - Added item_within_buffer_bounds? method to check if item fits in buffer - Updated comments in accessor classes to reflect new behavior Fixes #2463 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- openc3/ext/openc3/ext/structure/structure.c | 2 +- openc3/lib/openc3/accessors/accessor.rb | 2 +- .../lib/openc3/accessors/binary_accessor.rb | 40 +++++++++- openc3/lib/openc3/accessors/form_accessor.rb | 2 +- openc3/lib/openc3/accessors/http_accessor.rb | 2 +- openc3/lib/openc3/accessors/json_accessor.rb | 2 +- .../lib/openc3/accessors/template_accessor.rb | 2 +- openc3/lib/openc3/accessors/xml_accessor.rb | 2 +- openc3/lib/openc3/packets/packet_config.rb | 4 +- openc3/lib/openc3/packets/structure.rb | 11 ++- .../openc3/accessors/binary_accessor.py | 53 +++++++++++--- .../python/openc3/accessors/form_accessor.py | 2 +- .../python/openc3/accessors/http_accessor.py | 2 +- .../openc3/accessors/template_accessor.py | 2 +- openc3/python/openc3/packets/packet_config.py | 4 +- openc3/python/openc3/packets/structure.py | 5 +- .../accessors/test_binary_accessor_read.py | 72 +++++++++++++++++- openc3/python/test/packets/test_structure.py | 25 +++++++ openc3/spec/accessors/binary_accessor_spec.rb | 73 ++++++++++++++++++- openc3/spec/packets/structure_spec.rb | 26 +++++++ 20 files changed, 302 insertions(+), 31 deletions(-) diff --git a/openc3/ext/openc3/ext/structure/structure.c b/openc3/ext/openc3/ext/structure/structure.c index 048a396c3a..b7f57e7c1f 100644 --- a/openc3/ext/openc3/ext/structure/structure.c +++ b/openc3/ext/openc3/ext/structure/structure.c @@ -15,7 +15,7 @@ /* # Modified by OpenC3, Inc. -# All changes Copyright 2024, OpenC3, Inc. +# All changes Copyright 2025, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license diff --git a/openc3/lib/openc3/accessors/accessor.rb b/openc3/lib/openc3/accessors/accessor.rb index 60edad4cdf..1c24d661ce 100644 --- a/openc3/lib/openc3/accessors/accessor.rb +++ b/openc3/lib/openc3/accessors/accessor.rb @@ -85,7 +85,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return false end diff --git a/openc3/lib/openc3/accessors/binary_accessor.rb b/openc3/lib/openc3/accessors/binary_accessor.rb index 8c301303fc..5b09fde090 100644 --- a/openc3/lib/openc3/accessors/binary_accessor.rb +++ b/openc3/lib/openc3/accessors/binary_accessor.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2024, OpenC3, Inc. +# All changes Copyright 2025, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -106,6 +106,40 @@ def self.raise_buffer_error(read_write, buffer, data_type, given_bit_offset, giv raise ArgumentError, "#{buffer.length} byte buffer insufficient to #{read_write} #{data_type} at bit_offset #{given_bit_offset} with bit_size #{given_bit_size}" end + # Check if an item is completely within the buffer bounds + # Returns false if the item extends beyond the buffer, + # this is used to return nil for out-of-bounds reads + # + # @param item [StructureItem] The item to check + # @param buffer [String] The binary buffer + # @return [Boolean] true if the item is within buffer bounds, false otherwise + def self.item_within_buffer_bounds?(item, buffer) + return true if item.data_type == :DERIVED + + bit_offset = item.bit_offset + bit_size = item.bit_size + buffer_bits = buffer.length * 8 + + # Handle negative bit offsets (offset from end of buffer) + if bit_offset < 0 + bit_offset = buffer_bits + bit_offset + return false if bit_offset < 0 + end + + # For arrays, use array_size instead of bit_size + total_bits = item.array_size ? item.array_size : bit_size + + # Handle negative/zero bit sizes (fill to end of buffer) + if total_bits <= 0 + # These types fill to the end, so they're always "in bounds" if the starting position is valid + return bit_offset <= buffer_bits + end + + # Check if the item's upper bound is within buffer + upper_bound_bits = bit_offset + total_bits + return upper_bound_bits <= buffer_bits + end + # Store the host endianness so that it only has to be determined once HOST_ENDIANNESS = get_host_endianness() # Valid endianness @@ -294,6 +328,8 @@ def write_item(item, value, buffer) # Note: do not use directly - use instance read_item def self.read_item(item, buffer) return nil if item.data_type == :DERIVED + # Return nil if item is outside the buffer bounds (for undersized packets) + return nil unless item_within_buffer_bounds?(item, buffer) if item.array_size return read_array(item.bit_offset, item.bit_size, item.data_type, item.array_size, buffer, item.endianness) else @@ -1420,7 +1456,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return false end diff --git a/openc3/lib/openc3/accessors/form_accessor.rb b/openc3/lib/openc3/accessors/form_accessor.rb index 48c0f85b08..d6b2f7f553 100644 --- a/openc3/lib/openc3/accessors/form_accessor.rb +++ b/openc3/lib/openc3/accessors/form_accessor.rb @@ -75,7 +75,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return true end diff --git a/openc3/lib/openc3/accessors/http_accessor.rb b/openc3/lib/openc3/accessors/http_accessor.rb index 418a32f8f5..00fce951a4 100644 --- a/openc3/lib/openc3/accessors/http_accessor.rb +++ b/openc3/lib/openc3/accessors/http_accessor.rb @@ -164,7 +164,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return @body_accessor.enforce_short_buffer_allowed end diff --git a/openc3/lib/openc3/accessors/json_accessor.rb b/openc3/lib/openc3/accessors/json_accessor.rb index bcd4b1143d..d59fabf322 100644 --- a/openc3/lib/openc3/accessors/json_accessor.rb +++ b/openc3/lib/openc3/accessors/json_accessor.rb @@ -158,7 +158,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return true end diff --git a/openc3/lib/openc3/accessors/template_accessor.rb b/openc3/lib/openc3/accessors/template_accessor.rb index 69bd09ffe3..71e64ff45f 100644 --- a/openc3/lib/openc3/accessors/template_accessor.rb +++ b/openc3/lib/openc3/accessors/template_accessor.rb @@ -134,7 +134,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return true end diff --git a/openc3/lib/openc3/accessors/xml_accessor.rb b/openc3/lib/openc3/accessors/xml_accessor.rb index 4afd492848..11e46e1f5e 100644 --- a/openc3/lib/openc3/accessors/xml_accessor.rb +++ b/openc3/lib/openc3/accessors/xml_accessor.rb @@ -95,7 +95,7 @@ def enforce_length # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return nil when read. def enforce_short_buffer_allowed return true end diff --git a/openc3/lib/openc3/packets/packet_config.rb b/openc3/lib/openc3/packets/packet_config.rb index 0e67f775e0..84bc2a27e6 100644 --- a/openc3/lib/openc3/packets/packet_config.rb +++ b/openc3/lib/openc3/packets/packet_config.rb @@ -492,8 +492,8 @@ def process_current_packet(parser, keyword, params) 'APPEND_ARRAY_ITEM', 'APPEND_ARRAY_PARAMETER', 'STRUCTURE', 'APPEND_STRUCTURE' start_item(parser) - # Allow this packet to be received with less data than the defined length - # without generating a warning. + # Allow this packet to be received with less data than the defined length. + # Items that are beyond the buffer will return nil when read. when 'ALLOW_SHORT' @current_packet.short_buffer_allowed = true diff --git a/openc3/lib/openc3/packets/structure.rb b/openc3/lib/openc3/packets/structure.rb index a577be5ade..1c39cfe235 100644 --- a/openc3/lib/openc3/packets/structure.rb +++ b/openc3/lib/openc3/packets/structure.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2024, OpenC3, Inc. +# All changes Copyright 2025, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -680,8 +680,13 @@ def internal_buffer_equals(buffer) if @accessor.enforce_length if @buffer.length != @defined_length if @buffer.length < @defined_length - resize_buffer() - raise "Buffer length less than defined length" unless @short_buffer_allowed + # Only resize if short_buffer_allowed is false + # When short_buffer_allowed is true, keep the buffer short so reads + # of items beyond the buffer return nil + unless @short_buffer_allowed + resize_buffer() + raise "Buffer length less than defined length" + end elsif @fixed_size and @defined_length != 0 raise "Buffer length greater than defined length" end diff --git a/openc3/python/openc3/accessors/binary_accessor.py b/openc3/python/openc3/accessors/binary_accessor.py index 9f42959476..24845d14f7 100644 --- a/openc3/python/openc3/accessors/binary_accessor.py +++ b/openc3/python/openc3/accessors/binary_accessor.py @@ -99,13 +99,49 @@ def raise_buffer_error(cls, read_write, buffer, data_type, given_bit_offset, giv f"{len(buffer)} byte buffer insufficient to {read_write} {data_type} at bit_offset {given_bit_offset} with bit_size {given_bit_size}" ) + # Check if an item is completely within the buffer bounds + # Returns False if the item extends beyond the buffer, + # this is used to return None for out-of-bounds reads + # + # @param item [StructureItem] The item to check + # @param buffer [bytes/bytearray] The binary buffer + # @return [bool] True if the item is within buffer bounds, False otherwise + @classmethod + def item_within_buffer_bounds(cls, item, buffer): + if item.data_type == "DERIVED": + return True + + bit_offset = item.bit_offset + bit_size = item.bit_size + buffer_bits = len(buffer) * 8 + + # Handle negative bit offsets (offset from end of buffer) + if bit_offset < 0: + bit_offset = buffer_bits + bit_offset + if bit_offset < 0: + return False + + # For arrays, use array_size instead of bit_size + total_bits = item.array_size if item.array_size is not None else bit_size + + # Handle negative/zero bit sizes (fill to end of buffer) + if total_bits <= 0: + # These types fill to the end, so they're always "in bounds" if the starting position is valid + return bit_offset <= buffer_bits + + # Check if the item's upper bound is within buffer + upper_bound_bits = bit_offset + total_bits + return upper_bound_bits <= buffer_bits + # Valid endianness ENDIANNESS = ["BIG_ENDIAN", "LITTLE_ENDIAN"] def handle_read_variable_bit_size(self, item, _buffer): length_value = self.packet.read(item.variable_bit_size["length_item_name"], "CONVERTED") if item.array_size is not None: - item.array_size = (length_value * item.variable_bit_size["length_bits_per_count"]) + item.variable_bit_size["length_value_bit_offset"] + item.array_size = (length_value * item.variable_bit_size["length_bits_per_count"]) + item.variable_bit_size[ + "length_value_bit_offset" + ] else: if item.data_type == "INT" or item.data_type == "UINT": # QUIC encoding is currently assumed for individual variable sized integers @@ -133,7 +169,7 @@ def read_item(self, item, buffer): # Structure is used to read items with parent, not accessor structure_buffer = self.read_item(item.parent_item, buffer) structure = item.parent_item.structure - return structure.read(item.key, 'RAW', structure_buffer) + return structure.read(item.key, "RAW", structure_buffer) else: if item.variable_bit_size: self.handle_read_variable_bit_size(item, buffer) @@ -144,6 +180,9 @@ def read_item(self, item, buffer): def class_read_item(cls, item, buffer): if item.data_type == "DERIVED": return None + # Return None if item is outside the buffer bounds (for undersized packets) + if not cls.item_within_buffer_bounds(item, buffer): + return None if item.array_size is not None: return cls.read_array( item.bit_offset, @@ -291,7 +330,7 @@ def write_item(self, item, value, buffer): # Structure is used to write items with parent, not accessor structure_buffer = self.read_item(item.parent_item, buffer) structure = item.parent_item.structure - structure.write(item.key, value, 'RAW', structure_buffer) + structure.write(item.key, value, "RAW", structure_buffer) if item.parent_item.variable_bit_size: self.handle_write_variable_bit_size(item.parent_item, structure_buffer, buffer) BinaryAccessor.class_write_item(item.parent_item, structure_buffer, buffer) @@ -936,9 +975,7 @@ def read_array(cls, bit_offset, bit_size, data_type, array_size, buffer, endiann ) ) else: - raise ValueError( - f"bit_size is {given_bit_size} but must be 32 or 64 for data_type {data_type}" - ) + raise ValueError(f"bit_size is {given_bit_size} but must be 32 or 64 for data_type {data_type}") else: raise ValueError(f"bit_offset {given_bit_offset} is not byte aligned for data_type {data_type}") @@ -1149,9 +1186,7 @@ def write_array( *values, ) else: - raise ValueError( - f"bit_size is {given_bit_size} but must be 32 or 64 for data_type {data_type}" - ) + raise ValueError(f"bit_size is {given_bit_size} but must be 32 or 64 for data_type {data_type}") else: raise ValueError(f"bit_offset {given_bit_offset} is not byte aligned for data_type {data_type}") diff --git a/openc3/python/openc3/accessors/form_accessor.py b/openc3/python/openc3/accessors/form_accessor.py index 19fea78cce..5d3a35b16d 100644 --- a/openc3/python/openc3/accessors/form_accessor.py +++ b/openc3/python/openc3/accessors/form_accessor.py @@ -63,7 +63,7 @@ def enforce_length(self): # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return None when read. def enforce_short_buffer_allowed(self): return True diff --git a/openc3/python/openc3/accessors/http_accessor.py b/openc3/python/openc3/accessors/http_accessor.py index 7ecc228d61..52e5329ef4 100644 --- a/openc3/python/openc3/accessors/http_accessor.py +++ b/openc3/python/openc3/accessors/http_accessor.py @@ -163,7 +163,7 @@ def enforce_length(self): # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return None when read. def enforce_short_buffer_allowed(self): return self.body_accessor.enforce_short_buffer_allowed() diff --git a/openc3/python/openc3/accessors/template_accessor.py b/openc3/python/openc3/accessors/template_accessor.py index c2478dbd43..d2139cffa2 100644 --- a/openc3/python/openc3/accessors/template_accessor.py +++ b/openc3/python/openc3/accessors/template_accessor.py @@ -157,7 +157,7 @@ def enforce_length(self): # This sets the short_buffer_allowed flag in the Packet class # which allows packets that have a buffer shorter than the defined size. - # Note that the buffer is still resized to the defined length + # Items outside the buffer bounds will return None when read. def enforce_short_buffer_allowed(self): return True diff --git a/openc3/python/openc3/packets/packet_config.py b/openc3/python/openc3/packets/packet_config.py index 5392ae3fd6..58cbb9302f 100644 --- a/openc3/python/openc3/packets/packet_config.py +++ b/openc3/python/openc3/packets/packet_config.py @@ -594,8 +594,8 @@ def process_current_packet(self, parser, keyword, params): ): self.start_item(parser) - # Allow this packet to be received with less data than the defined length - # without generating a warning. + # Allow this packet to be received with less data than the defined length. + # Items that are beyond the buffer will return None when read. case "ALLOW_SHORT": usage = keyword parser.verify_num_parameters(0, 0, usage) diff --git a/openc3/python/openc3/packets/structure.py b/openc3/python/openc3/packets/structure.py index d73bb47598..0e9a15ce65 100644 --- a/openc3/python/openc3/packets/structure.py +++ b/openc3/python/openc3/packets/structure.py @@ -605,8 +605,11 @@ def internal_buffer_equals(self, buffer): if self.accessor.enforce_length(): if len(self._buffer) != self.defined_length: if len(self._buffer) < self.defined_length: - self.resize_buffer() + # Only resize if short_buffer_allowed is false + # When short_buffer_allowed is true, keep the buffer short so reads + # of items beyond the buffer return None if not self.short_buffer_allowed: + self.resize_buffer() raise ValueError("Buffer length less than defined length") elif self.fixed_size and self.defined_length != 0: raise ValueError("Buffer length greater than defined length") diff --git a/openc3/python/test/accessors/test_binary_accessor_read.py b/openc3/python/test/accessors/test_binary_accessor_read.py index b145aba468..9afb69c794 100644 --- a/openc3/python/test/accessors/test_binary_accessor_read.py +++ b/openc3/python/test/accessors/test_binary_accessor_read.py @@ -1,4 +1,4 @@ -# Copyright 2023 OpenC3, Inc. +# Copyright 2025 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -1114,3 +1114,73 @@ def test_reads_aligned_64_bit_floats(self): actual = BinaryAccessor.read_array(0, 64, "FLOAT", 0, self.data, "BIG_ENDIAN") for index, val in enumerate(actual): self.assertAlmostEqual(val, expected_array[index]) + + +class TestBinaryAccessorItemWithinBufferBounds(unittest.TestCase): + def test_returns_true_for_items_within_buffer_bounds(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03\x04\x05\x06\x07" + item = StructureItem("TEST", 0, 32, "UINT", "BIG_ENDIAN") + self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + def test_returns_false_for_items_that_exceed_buffer_bounds(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem("TEST", 0, 64, "UINT", "BIG_ENDIAN") + self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + def test_returns_false_for_items_with_offset_outside_buffer(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem("TEST", 64, 8, "UINT", "BIG_ENDIAN") + self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + def test_returns_true_for_derived_items_regardless_of_buffer_size(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00" + item = StructureItem("TEST", 0, 0, "DERIVED", "BIG_ENDIAN") + self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + def test_handles_negative_bit_offsets_correctly(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem("TEST", -8, 8, "UINT", "BIG_ENDIAN") + self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + item2 = StructureItem("TEST2", -64, 8, "UINT", "BIG_ENDIAN") + self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item2, buffer)) + + def test_handles_zero_negative_bit_sizes_fill_to_end(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03" + item = StructureItem("TEST", 8, 0, "STRING", "BIG_ENDIAN") + self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + def test_handles_arrays_correctly(self): + from openc3.packets.structure_item import StructureItem + buffer = b"\x00\x01\x02\x03\x04\x05\x06\x07" + item = StructureItem("TEST", 0, 8, "UINT", "BIG_ENDIAN", 32) + self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) + + item2 = StructureItem("TEST2", 0, 8, "UINT", "BIG_ENDIAN", 128) + self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item2, buffer)) + + +class TestBinaryAccessorReadItemUndersizedBuffer(unittest.TestCase): + def test_returns_none_for_items_outside_buffer_bounds(self): + from openc3.packets.structure_item import StructureItem + buffer = bytearray(b"\x00\x01\x02\x03") # 4 bytes + item = StructureItem("TEST", 32, 32, "UINT", "BIG_ENDIAN") + self.assertIsNone(BinaryAccessor.class_read_item(item, buffer)) + + def test_returns_value_for_items_within_buffer_bounds(self): + from openc3.packets.structure_item import StructureItem + buffer = bytearray(b"\x00\x01\x02\x03") # 4 bytes + item = StructureItem("TEST", 0, 32, "UINT", "BIG_ENDIAN") + self.assertEqual(BinaryAccessor.class_read_item(item, buffer), 0x00010203) + + def test_returns_none_for_partially_out_of_bounds_items(self): + from openc3.packets.structure_item import StructureItem + buffer = bytearray(b"\x00\x01\x02\x03") # 4 bytes = 32 bits + item = StructureItem("TEST", 16, 32, "UINT", "BIG_ENDIAN") + self.assertIsNone(BinaryAccessor.class_read_item(item, buffer)) diff --git a/openc3/python/test/packets/test_structure.py b/openc3/python/test/packets/test_structure.py index 926fe753ef..db755675a5 100644 --- a/openc3/python/test/packets/test_structure.py +++ b/openc3/python/test/packets/test_structure.py @@ -1129,3 +1129,28 @@ def test_recalculates_the_bit_offsets_for_0_size(self): self.assertEqual(s.read("test1"), b"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09") self.assertEqual(s.read("test2"), b"\x0a\x0b\x0c\x0d\x0e\x0f\x0f\x0e\x0d\x0c\x0b\x0a\xaa\x55") self.assertEqual(s.read("test3"), 0xAA55) + + +class TestStructureShortBufferAllowed(unittest.TestCase): + def test_returns_none_for_items_outside_buffer_bounds(self): + s = Structure("BIG_ENDIAN") + s.append_item("item1", 16, "UINT") + s.append_item("item2", 16, "UINT") + s.append_item("item3", 16, "UINT") + s.short_buffer_allowed = True + # Set a short buffer that only contains data for item1 + s.buffer = b"\x00\x01" + # item1 should read successfully + self.assertEqual(s.read("item1"), 1) + # item2 and item3 should return None since they're outside the buffer + self.assertIsNone(s.read("item2")) + self.assertIsNone(s.read("item3")) + # Buffer should remain at its original size (not padded) + self.assertEqual(len(s.buffer), 2) + + def test_raises_error_when_short_buffer_allowed_is_false(self): + s = Structure("BIG_ENDIAN") + s.append_item("item1", 16, "UINT") + s.append_item("item2", 16, "UINT") + with self.assertRaisesRegex(ValueError, "Buffer length less than defined length"): + s.buffer = b"\x00\x01" diff --git a/openc3/spec/accessors/binary_accessor_spec.rb b/openc3/spec/accessors/binary_accessor_spec.rb index 50f9a8de4d..aab33b6a7e 100644 --- a/openc3/spec/accessors/binary_accessor_spec.rb +++ b/openc3/spec/accessors/binary_accessor_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2022, OpenC3, Inc. +# All changes Copyright 2025, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -24,6 +24,7 @@ require 'openc3' require 'openc3/accessors/binary_accessor' require 'openc3/packets/packet' +require 'openc3/packets/structure_item' module OpenC3 describe BinaryAccessor, no_ext: true do @@ -2564,5 +2565,75 @@ module OpenC3 end end end # describe "write_array" + + describe "item_within_buffer_bounds?" do + it "returns true for items within the buffer bounds" do + buffer = "\x00\x01\x02\x03\x04\x05\x06\x07" + item = StructureItem.new("TEST", 0, 32, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true + end + + it "returns false for items that exceed buffer bounds" do + buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem.new("TEST", 0, 64, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be false + end + + it "returns false for items with offset outside buffer" do + buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem.new("TEST", 64, 8, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be false + end + + it "returns true for derived items regardless of buffer size" do + buffer = "\x00" + item = StructureItem.new("TEST", 0, 0, :DERIVED, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true + end + + it "handles negative bit offsets correctly" do + buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem.new("TEST", -8, 8, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true + + item2 = StructureItem.new("TEST2", -64, 8, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item2, buffer)).to be false + end + + it "handles zero/negative bit sizes (fill to end)" do + buffer = "\x00\x01\x02\x03" + item = StructureItem.new("TEST", 8, 0, :STRING, :BIG_ENDIAN) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true + end + + it "handles arrays correctly" do + buffer = "\x00\x01\x02\x03\x04\x05\x06\x07" + item = StructureItem.new("TEST", 0, 8, :UINT, :BIG_ENDIAN, 32) + expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true + + item2 = StructureItem.new("TEST2", 0, 8, :UINT, :BIG_ENDIAN, 128) + expect(BinaryAccessor.item_within_buffer_bounds?(item2, buffer)).to be false + end + end + + describe "read_item with undersized buffer" do + it "returns nil for items outside buffer bounds" do + buffer = "\x00\x01\x02\x03" # 4 bytes + item = StructureItem.new("TEST", 32, 32, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.read_item(item, buffer)).to be_nil + end + + it "returns value for items within buffer bounds" do + buffer = "\x00\x01\x02\x03" # 4 bytes + item = StructureItem.new("TEST", 0, 32, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.read_item(item, buffer)).to eq(0x00010203) + end + + it "returns nil for partially out-of-bounds items" do + buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits + item = StructureItem.new("TEST", 16, 32, :UINT, :BIG_ENDIAN) + expect(BinaryAccessor.read_item(item, buffer)).to be_nil + end + end end # describe BinaryAccessor end diff --git a/openc3/spec/packets/structure_spec.rb b/openc3/spec/packets/structure_spec.rb index 6b2242d428..048f321127 100644 --- a/openc3/spec/packets/structure_spec.rb +++ b/openc3/spec/packets/structure_spec.rb @@ -892,5 +892,31 @@ module OpenC3 s.set_item(item) end end + + describe "short_buffer_allowed" do + it "returns nil for items outside buffer bounds when short_buffer_allowed is true" do + s = Structure.new(:BIG_ENDIAN) + s.append_item("item1", 16, :UINT) + s.append_item("item2", 16, :UINT) + s.append_item("item3", 16, :UINT) + s.short_buffer_allowed = true + # Set a short buffer that only contains data for item1 + s.buffer = "\x00\x01" + # item1 should read successfully + expect(s.read("item1")).to eq(1) + # item2 and item3 should return nil since they're outside the buffer + expect(s.read("item2")).to be_nil + expect(s.read("item3")).to be_nil + # Buffer should remain at its original size (not padded) + expect(s.buffer.length).to eq(2) + end + + it "raises error when short_buffer_allowed is false" do + s = Structure.new(:BIG_ENDIAN) + s.append_item("item1", 16, :UINT) + s.append_item("item2", 16, :UINT) + expect { s.buffer = "\x00\x01" }.to raise_error(RuntimeError, /Buffer length less than defined length/) + end + end end # describe Structure end From 975ce22b4449b22509d82f8efa5bec0de92a4d6d Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Fri, 9 Jan 2026 10:12:28 -0700 Subject: [PATCH 2/6] Move bounds check into low-level read code for efficiency Address PR review comment by moving the bounds checking logic into the existing low-level read functions instead of having a separate item_within_buffer_bounds? check. This eliminates duplicate bounds checking and keeps the check in the C code for better performance. Changes: - C code returns nil instead of raising on bounds error - Ruby/Python read methods return nil on bounds failure - Added bounds check to read_array to return nil for undersized buffers - Removed item_within_buffer_bounds method and its call from read_item - Removed unnecessary resize_buffer call in structure files - Updated tests to expect nil instead of errors for out-of-bounds reads Co-Authored-By: Claude Opus 4.5 --- openc3/ext/openc3/ext/structure/structure.c | 5 +- .../lib/openc3/accessors/binary_accessor.rb | 44 ++--------- openc3/lib/openc3/packets/structure.rb | 3 +- .../openc3/accessors/binary_accessor.py | 46 ++---------- openc3/python/openc3/packets/structure.py | 10 ++- .../accessors/test_binary_accessor_read.py | 73 ++----------------- openc3/spec/accessors/binary_accessor_spec.rb | 64 ++-------------- 7 files changed, 36 insertions(+), 209 deletions(-) diff --git a/openc3/ext/openc3/ext/structure/structure.c b/openc3/ext/openc3/ext/structure/structure.c index b7f57e7c1f..f985a324e3 100644 --- a/openc3/ext/openc3/ext/structure/structure.c +++ b/openc3/ext/openc3/ext/structure/structure.c @@ -15,7 +15,7 @@ /* # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -520,7 +520,8 @@ static VALUE binary_accessor_read(VALUE self, VALUE param_bit_offset, VALUE para if (!check_bounds_and_buffer_size(bit_offset, bit_size, (int)buffer_length, param_endianness, param_data_type, &lower_bound, &upper_bound)) { - rb_funcall(self, id_method_raise_buffer_error, 5, symbol_read, param_buffer, param_data_type, param_bit_offset, param_bit_size); + /* Return nil for out-of-bounds reads (supports undersized packets with ALLOW_SHORT) */ + return Qnil; } if ((param_data_type == symbol_STRING) || (param_data_type == symbol_BLOCK)) diff --git a/openc3/lib/openc3/accessors/binary_accessor.rb b/openc3/lib/openc3/accessors/binary_accessor.rb index 5b09fde090..993a1c44ab 100644 --- a/openc3/lib/openc3/accessors/binary_accessor.rb +++ b/openc3/lib/openc3/accessors/binary_accessor.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -106,40 +106,6 @@ def self.raise_buffer_error(read_write, buffer, data_type, given_bit_offset, giv raise ArgumentError, "#{buffer.length} byte buffer insufficient to #{read_write} #{data_type} at bit_offset #{given_bit_offset} with bit_size #{given_bit_size}" end - # Check if an item is completely within the buffer bounds - # Returns false if the item extends beyond the buffer, - # this is used to return nil for out-of-bounds reads - # - # @param item [StructureItem] The item to check - # @param buffer [String] The binary buffer - # @return [Boolean] true if the item is within buffer bounds, false otherwise - def self.item_within_buffer_bounds?(item, buffer) - return true if item.data_type == :DERIVED - - bit_offset = item.bit_offset - bit_size = item.bit_size - buffer_bits = buffer.length * 8 - - # Handle negative bit offsets (offset from end of buffer) - if bit_offset < 0 - bit_offset = buffer_bits + bit_offset - return false if bit_offset < 0 - end - - # For arrays, use array_size instead of bit_size - total_bits = item.array_size ? item.array_size : bit_size - - # Handle negative/zero bit sizes (fill to end of buffer) - if total_bits <= 0 - # These types fill to the end, so they're always "in bounds" if the starting position is valid - return bit_offset <= buffer_bits - end - - # Check if the item's upper bound is within buffer - upper_bound_bits = bit_offset + total_bits - return upper_bound_bits <= buffer_bits - end - # Store the host endianness so that it only has to be determined once HOST_ENDIANNESS = get_host_endianness() # Valid endianness @@ -328,8 +294,6 @@ def write_item(item, value, buffer) # Note: do not use directly - use instance read_item def self.read_item(item, buffer) return nil if item.data_type == :DERIVED - # Return nil if item is outside the buffer bounds (for undersized packets) - return nil unless item_within_buffer_bounds?(item, buffer) if item.array_size return read_array(item.bit_offset, item.bit_size, item.data_type, item.array_size, buffer, item.endianness) else @@ -375,7 +339,8 @@ def self.read(bit_offset, bit_size, data_type, buffer, endianness) end result, lower_bound, upper_bound = check_bounds_and_buffer_size(bit_offset, bit_size, buffer.length, endianness, data_type) - raise_buffer_error(:read, buffer, data_type, given_bit_offset, given_bit_size) unless result + # Return nil for out-of-bounds reads (supports undersized packets with ALLOW_SHORT) + return nil unless result if (data_type == :STRING) || (data_type == :BLOCK) ####################################### @@ -972,6 +937,9 @@ def self.read_array(bit_offset, bit_size, data_type, array_size, buffer, endiann lower_bound = bit_offset / 8 upper_bound = (bit_offset + array_size - 1) / 8 + # Return nil for out-of-bounds reads (supports undersized packets with ALLOW_SHORT) + return nil if upper_bound >= buffer.length + # Check for byte alignment byte_aligned = ((bit_offset % 8) == 0) diff --git a/openc3/lib/openc3/packets/structure.rb b/openc3/lib/openc3/packets/structure.rb index 1c39cfe235..e843f3d2fc 100644 --- a/openc3/lib/openc3/packets/structure.rb +++ b/openc3/lib/openc3/packets/structure.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -684,7 +684,6 @@ def internal_buffer_equals(buffer) # When short_buffer_allowed is true, keep the buffer short so reads # of items beyond the buffer return nil unless @short_buffer_allowed - resize_buffer() raise "Buffer length less than defined length" end elsif @fixed_size and @defined_length != 0 diff --git a/openc3/python/openc3/accessors/binary_accessor.py b/openc3/python/openc3/accessors/binary_accessor.py index 24845d14f7..dcc7ae7e01 100644 --- a/openc3/python/openc3/accessors/binary_accessor.py +++ b/openc3/python/openc3/accessors/binary_accessor.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -99,40 +99,6 @@ def raise_buffer_error(cls, read_write, buffer, data_type, given_bit_offset, giv f"{len(buffer)} byte buffer insufficient to {read_write} {data_type} at bit_offset {given_bit_offset} with bit_size {given_bit_size}" ) - # Check if an item is completely within the buffer bounds - # Returns False if the item extends beyond the buffer, - # this is used to return None for out-of-bounds reads - # - # @param item [StructureItem] The item to check - # @param buffer [bytes/bytearray] The binary buffer - # @return [bool] True if the item is within buffer bounds, False otherwise - @classmethod - def item_within_buffer_bounds(cls, item, buffer): - if item.data_type == "DERIVED": - return True - - bit_offset = item.bit_offset - bit_size = item.bit_size - buffer_bits = len(buffer) * 8 - - # Handle negative bit offsets (offset from end of buffer) - if bit_offset < 0: - bit_offset = buffer_bits + bit_offset - if bit_offset < 0: - return False - - # For arrays, use array_size instead of bit_size - total_bits = item.array_size if item.array_size is not None else bit_size - - # Handle negative/zero bit sizes (fill to end of buffer) - if total_bits <= 0: - # These types fill to the end, so they're always "in bounds" if the starting position is valid - return bit_offset <= buffer_bits - - # Check if the item's upper bound is within buffer - upper_bound_bits = bit_offset + total_bits - return upper_bound_bits <= buffer_bits - # Valid endianness ENDIANNESS = ["BIG_ENDIAN", "LITTLE_ENDIAN"] @@ -180,9 +146,6 @@ def read_item(self, item, buffer): def class_read_item(cls, item, buffer): if item.data_type == "DERIVED": return None - # Return None if item is outside the buffer bounds (for undersized packets) - if not cls.item_within_buffer_bounds(item, buffer): - return None if item.array_size is not None: return cls.read_array( item.bit_offset, @@ -394,8 +357,9 @@ def read(cls, bit_offset, bit_size, data_type, buffer, endianness): result, lower_bound, upper_bound = cls.check_bounds_and_buffer_size( bit_offset, bit_size, len(buffer), endianness, data_type ) + # Return None for out-of-bounds reads (supports undersized packets with ALLOW_SHORT) if not result: - cls.raise_buffer_error("read", buffer, data_type, given_bit_offset, given_bit_size) + return None if data_type in ["STRING", "BLOCK"]: ####################################### @@ -910,6 +874,10 @@ def read_array(cls, bit_offset, bit_size, data_type, array_size, buffer, endiann lower_bound = math.floor(bit_offset / 8) upper_bound = math.floor((bit_offset + array_size - 1) / 8) + # Return None for out-of-bounds reads (supports undersized packets with ALLOW_SHORT) + if upper_bound >= len(buffer): + return None + # Check for byte alignment byte_aligned = (bit_offset % 8) == 0 diff --git a/openc3/python/openc3/packets/structure.py b/openc3/python/openc3/packets/structure.py index 0e9a15ce65..cc73214ae9 100644 --- a/openc3/python/openc3/packets/structure.py +++ b/openc3/python/openc3/packets/structure.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -327,7 +327,12 @@ def set_item(self, item): item.variable_bit_size["length_value_bit_offset"] * item.variable_bit_size["length_bits_per_count"] ) - if minimum_data_bits > 0 and item.bit_offset >= 0 and self.defined_length_bits == item.bit_offset and item.parent_item is None: + if ( + minimum_data_bits > 0 + and item.bit_offset >= 0 + and self.defined_length_bits == item.bit_offset + and item.parent_item is None + ): self.defined_length_bits += minimum_data_bits else: raise ValueError(f"Unknown item: {item.name} - Ensure item name is uppercase") @@ -609,7 +614,6 @@ def internal_buffer_equals(self, buffer): # When short_buffer_allowed is true, keep the buffer short so reads # of items beyond the buffer return None if not self.short_buffer_allowed: - self.resize_buffer() raise ValueError("Buffer length less than defined length") elif self.fixed_size and self.defined_length != 0: raise ValueError("Buffer length greater than defined length") diff --git a/openc3/python/test/accessors/test_binary_accessor_read.py b/openc3/python/test/accessors/test_binary_accessor_read.py index 9afb69c794..63adde6039 100644 --- a/openc3/python/test/accessors/test_binary_accessor_read.py +++ b/openc3/python/test/accessors/test_binary_accessor_read.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -191,17 +191,8 @@ def test_complains_about_unaligned_blocks(self): "BIG_ENDIAN", ) - def test_complains_if_read_exceeds_the_size_of_the_buffer(self): - self.assertRaisesRegex( - ValueError, - "16 byte buffer insufficient to read STRING at bit_offset 8 with bit_size 800", - BinaryAccessor.read, - 8, - 800, - "STRING", - self.data, - "BIG_ENDIAN", - ) + def test_returns_none_if_read_exceeds_the_size_of_the_buffer(self): + self.assertIsNone(BinaryAccessor.read(8, 800, "STRING", self.data, "BIG_ENDIAN")) def test_reads_aligned_8_bit_unsigned_integers(self): for bit_offset in range(0, (len(self.data) - 1) * 8, 8): @@ -949,12 +940,8 @@ def test_complains_with_negative_array_size(self): ): BinaryAccessor.read_array(-32, 8, "UINT", -8, self.data, "LITTLE_ENDIAN") - def test_complains_about_accessing_data_from_a_buffer_which_is_too_small(self): - with self.assertRaisesRegex( - ValueError, - "16 byte buffer insufficient to read STRING at bit_offset 0 with bit_size 256", - ): - BinaryAccessor.read_array(0, 256, "STRING", 256, self.data, "LITTLE_ENDIAN") + def test_returns_none_for_accessing_data_from_a_buffer_which_is_too_small(self): + self.assertIsNone(BinaryAccessor.read_array(0, 256, "STRING", 256, self.data, "LITTLE_ENDIAN")) def test_returns_an_empty_array_when_passed_a_zero_length_buffer(self): self.assertEqual(BinaryAccessor.read_array(0, 8, "UINT", 32, b"", "LITTLE_ENDIAN"), []) @@ -1116,56 +1103,6 @@ def test_reads_aligned_64_bit_floats(self): self.assertAlmostEqual(val, expected_array[index]) -class TestBinaryAccessorItemWithinBufferBounds(unittest.TestCase): - def test_returns_true_for_items_within_buffer_bounds(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03\x04\x05\x06\x07" - item = StructureItem("TEST", 0, 32, "UINT", "BIG_ENDIAN") - self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - def test_returns_false_for_items_that_exceed_buffer_bounds(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem("TEST", 0, 64, "UINT", "BIG_ENDIAN") - self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - def test_returns_false_for_items_with_offset_outside_buffer(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem("TEST", 64, 8, "UINT", "BIG_ENDIAN") - self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - def test_returns_true_for_derived_items_regardless_of_buffer_size(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00" - item = StructureItem("TEST", 0, 0, "DERIVED", "BIG_ENDIAN") - self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - def test_handles_negative_bit_offsets_correctly(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem("TEST", -8, 8, "UINT", "BIG_ENDIAN") - self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - item2 = StructureItem("TEST2", -64, 8, "UINT", "BIG_ENDIAN") - self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item2, buffer)) - - def test_handles_zero_negative_bit_sizes_fill_to_end(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03" - item = StructureItem("TEST", 8, 0, "STRING", "BIG_ENDIAN") - self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - def test_handles_arrays_correctly(self): - from openc3.packets.structure_item import StructureItem - buffer = b"\x00\x01\x02\x03\x04\x05\x06\x07" - item = StructureItem("TEST", 0, 8, "UINT", "BIG_ENDIAN", 32) - self.assertTrue(BinaryAccessor.item_within_buffer_bounds(item, buffer)) - - item2 = StructureItem("TEST2", 0, 8, "UINT", "BIG_ENDIAN", 128) - self.assertFalse(BinaryAccessor.item_within_buffer_bounds(item2, buffer)) - - class TestBinaryAccessorReadItemUndersizedBuffer(unittest.TestCase): def test_returns_none_for_items_outside_buffer_bounds(self): from openc3.packets.structure_item import StructureItem diff --git a/openc3/spec/accessors/binary_accessor_spec.rb b/openc3/spec/accessors/binary_accessor_spec.rb index aab33b6a7e..05ffe74e03 100644 --- a/openc3/spec/accessors/binary_accessor_spec.rb +++ b/openc3/spec/accessors/binary_accessor_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -366,8 +366,8 @@ module OpenC3 expect { BinaryAccessor.read(7, 16, :BLOCK, @data, :BIG_ENDIAN) }.to raise_error(ArgumentError, "bit_offset 7 is not byte aligned for data_type BLOCK") end - it "complains if read exceeds the size of the buffer" do - expect { BinaryAccessor.read(8, 800, :STRING, @data, :BIG_ENDIAN) }.to raise_error(ArgumentError, "16 byte buffer insufficient to read STRING at bit_offset 8 with bit_size 800") + it "returns nil if read exceeds the size of the buffer" do + expect(BinaryAccessor.read(8, 800, :STRING, @data, :BIG_ENDIAN)).to be_nil end it "reads aligned 8-bit unsigned integers" do @@ -815,12 +815,12 @@ module OpenC3 end end - it "complains about accessing data from a buffer which is too small" do - expect { BinaryAccessor.read_array(0, 256, :STRING, 256, @data, :LITTLE_ENDIAN) }.to raise_error(ArgumentError, "16 byte buffer insufficient to read STRING at bit_offset 0 with bit_size 256") + it "returns nil for accessing data from a buffer which is too small" do + expect(BinaryAccessor.read_array(0, 256, :STRING, 256, @data, :LITTLE_ENDIAN)).to be_nil end - it "returns an empty array when passed a zero length buffer" do - expect(BinaryAccessor.read_array(0, 8, :UINT, 32, "", :LITTLE_ENDIAN)).to eql([]) + it "returns nil when reading beyond an empty buffer" do + expect(BinaryAccessor.read_array(0, 8, :UINT, 32, "", :LITTLE_ENDIAN)).to be_nil end it "complains about unaligned strings" do @@ -2566,56 +2566,6 @@ module OpenC3 end end # describe "write_array" - describe "item_within_buffer_bounds?" do - it "returns true for items within the buffer bounds" do - buffer = "\x00\x01\x02\x03\x04\x05\x06\x07" - item = StructureItem.new("TEST", 0, 32, :UINT, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true - end - - it "returns false for items that exceed buffer bounds" do - buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem.new("TEST", 0, 64, :UINT, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be false - end - - it "returns false for items with offset outside buffer" do - buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem.new("TEST", 64, 8, :UINT, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be false - end - - it "returns true for derived items regardless of buffer size" do - buffer = "\x00" - item = StructureItem.new("TEST", 0, 0, :DERIVED, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true - end - - it "handles negative bit offsets correctly" do - buffer = "\x00\x01\x02\x03" # 4 bytes = 32 bits - item = StructureItem.new("TEST", -8, 8, :UINT, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true - - item2 = StructureItem.new("TEST2", -64, 8, :UINT, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item2, buffer)).to be false - end - - it "handles zero/negative bit sizes (fill to end)" do - buffer = "\x00\x01\x02\x03" - item = StructureItem.new("TEST", 8, 0, :STRING, :BIG_ENDIAN) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true - end - - it "handles arrays correctly" do - buffer = "\x00\x01\x02\x03\x04\x05\x06\x07" - item = StructureItem.new("TEST", 0, 8, :UINT, :BIG_ENDIAN, 32) - expect(BinaryAccessor.item_within_buffer_bounds?(item, buffer)).to be true - - item2 = StructureItem.new("TEST2", 0, 8, :UINT, :BIG_ENDIAN, 128) - expect(BinaryAccessor.item_within_buffer_bounds?(item2, buffer)).to be false - end - end - describe "read_item with undersized buffer" do it "returns nil for items outside buffer bounds" do buffer = "\x00\x01\x02\x03" # 4 bytes From 2fc46a6806b5d78e6f9e10b8c41e7c23debbf719 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Tue, 13 Jan 2026 20:40:49 -0700 Subject: [PATCH 3/6] Handle nil values in conversions and limits checking for undersized packets Add nil guards to prevent errors when items return nil for being outside buffer bounds in undersized packets: - Add nil guard in check_limits to skip limits checking for nil values - Add nil guard in apply_format_string_and_units to return nil early - Add nil guard in polynomial_conversion (Ruby, C extension, and Python) - Add nil guard in segmented_polynomial_conversion (Ruby and Python) Update tests to use properly sized buffers or expect nil for out-of-bounds items. Co-Authored-By: Claude Opus 4.5 --- .../polynomial_conversion.c | 17 +++++++++++++---- .../openc3/conversions/polynomial_conversion.rb | 5 ++++- .../segmented_polynomial_conversion.rb | 5 ++++- openc3/lib/openc3/packets/packet.rb | 8 +++++++- .../openc3/conversions/polynomial_conversion.py | 6 +++++- .../segmented_polynomial_conversion.py | 6 +++++- openc3/python/openc3/packets/packet.py | 10 +++++++++- .../test/packets/parsers/test_limits_parser.py | 6 +++--- openc3/python/test/packets/test_commands.py | 4 ++-- openc3/python/test/packets/test_packet.py | 4 ++-- .../python/test/packets/test_packet_config.py | 12 ++++++------ openc3/spec/packets/commands_spec.rb | 4 ++-- openc3/spec/packets/packet_config_spec.rb | 12 ++++++------ openc3/spec/packets/packet_spec.rb | 14 +++++++------- .../spec/packets/parsers/limits_parser_spec.rb | 6 +++--- 15 files changed, 78 insertions(+), 41 deletions(-) diff --git a/openc3/ext/openc3/ext/polynomial_conversion/polynomial_conversion.c b/openc3/ext/openc3/ext/polynomial_conversion/polynomial_conversion.c index 1a821b60b3..b53e7cbe44 100644 --- a/openc3/ext/openc3/ext/polynomial_conversion/polynomial_conversion.c +++ b/openc3/ext/openc3/ext/polynomial_conversion/polynomial_conversion.c @@ -15,7 +15,7 @@ /* # Modified by OpenC3, Inc. -# All changes Copyright 2022, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -47,9 +47,18 @@ static VALUE polynomial_conversion_call(VALUE self, VALUE value, VALUE myself, V int index = 0; double converted = 0.0; double raised_to_power = 1.0; - volatile VALUE coeffs = rb_ivar_get(self, id_ivar_coeffs); - long coeffs_length = RARRAY_LEN(coeffs); - double double_value = RFLOAT_VALUE(rb_funcall(value, id_method_to_f, 0)); + volatile VALUE coeffs; + long coeffs_length; + double double_value; + + /* Return nil if value is nil (item outside buffer bounds) */ + if (NIL_P(value)) { + return Qnil; + } + + coeffs = rb_ivar_get(self, id_ivar_coeffs); + coeffs_length = RARRAY_LEN(coeffs); + double_value = RFLOAT_VALUE(rb_funcall(value, id_method_to_f, 0)); /* Handle C0 */ double coeff = RFLOAT_VALUE(rb_ary_entry(coeffs, 0)); diff --git a/openc3/lib/openc3/conversions/polynomial_conversion.rb b/openc3/lib/openc3/conversions/polynomial_conversion.rb index e91b0a31ed..dd49ed4785 100644 --- a/openc3/lib/openc3/conversions/polynomial_conversion.rb +++ b/openc3/lib/openc3/conversions/polynomial_conversion.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2024, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -45,6 +45,9 @@ def initialize(*coeffs) # @param (see Conversion#call) # @return [Float] The value with the polynomial applied def call(value, myself, buffer) + # Return nil if value is nil (item outside buffer bounds) + return nil if value.nil? + value = value.to_f # Handle C0 diff --git a/openc3/lib/openc3/conversions/segmented_polynomial_conversion.rb b/openc3/lib/openc3/conversions/segmented_polynomial_conversion.rb index 95d16057e1..89b3e5c4cb 100644 --- a/openc3/lib/openc3/conversions/segmented_polynomial_conversion.rb +++ b/openc3/lib/openc3/conversions/segmented_polynomial_conversion.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2024, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -116,6 +116,9 @@ def add_segment(lower_bound, *coeffs) # @param (see Conversion#call) # @return [Float] The value with the polynomial applied def call(value, packet, buffer) + # Return nil if value is nil (item outside buffer bounds) + return nil if value.nil? + # Try to find correct segment @segments.each do |segment| return segment.calculate(value) if value >= segment.lower_bound diff --git a/openc3/lib/openc3/packets/packet.rb b/openc3/lib/openc3/packets/packet.rb index 457ce1f2a6..fb71026666 100644 --- a/openc3/lib/openc3/packets/packet.rb +++ b/openc3/lib/openc3/packets/packet.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -1048,6 +1048,9 @@ def check_limits(limits_set = :DEFAULT, ignore_persistence = false) if item.limits.enabled value = read_item(item) + # Skip limits checking if value is nil (item outside buffer bounds) + next if value.nil? + # Handle state monitoring and value monitoring differently if item.states handle_limits_states(item, value) @@ -1520,6 +1523,9 @@ def handle_limits_values(item, value, limits_set, ignore_persistence) end def apply_format_string_and_units(item, value, value_type) + # Return nil as-is - can't format a value that doesn't exist + return nil if value.nil? + if value_type == :FORMATTED or value_type == :WITH_UNITS if item.format_string && value value = sprintf(item.format_string, value) diff --git a/openc3/python/openc3/conversions/polynomial_conversion.py b/openc3/python/openc3/conversions/polynomial_conversion.py index 59297881cc..bebedc24b1 100644 --- a/openc3/python/openc3/conversions/polynomial_conversion.py +++ b/openc3/python/openc3/conversions/polynomial_conversion.py @@ -1,4 +1,4 @@ -# Copyright 2024 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -34,6 +34,10 @@ def __init__(self, *coeffs): # @param (see Conversion#call) # @return [Float] The value with the polynomial applied def call(self, value, myself, buffer): + # Return None if value is None (item outside buffer bounds) + if value is None: + return None + value = float(value) # Handle C0 diff --git a/openc3/python/openc3/conversions/segmented_polynomial_conversion.py b/openc3/python/openc3/conversions/segmented_polynomial_conversion.py index 96e45f08cc..0231e4669d 100644 --- a/openc3/python/openc3/conversions/segmented_polynomial_conversion.py +++ b/openc3/python/openc3/conversions/segmented_polynomial_conversion.py @@ -1,4 +1,4 @@ -# Copyright 2023 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -92,6 +92,10 @@ def add_segment(self, lower_bound, *coeffs): # @param (see Conversion#call) # @return [Float] The value with the polynomial applied def call(self, value, packet, buffer): + # Return None if value is None (item outside buffer bounds) + if value is None: + return None + # Try to find correct segment for segment in self.segments: if value >= segment.lower_bound: diff --git a/openc3/python/openc3/packets/packet.py b/openc3/python/openc3/packets/packet.py index cb39500a45..ad73f33276 100644 --- a/openc3/python/openc3/packets/packet.py +++ b/openc3/python/openc3/packets/packet.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -979,6 +979,10 @@ def check_limits(self, limits_set="DEFAULT", ignore_persistence=False): if item.limits.enabled: value = self.read_item(item) + # Skip limits checking if value is None (item outside buffer bounds) + if value is None: + continue + # Handle state monitoring and value monitoring differently if item.states is not None: self.handle_limits_states(item, value) @@ -1314,6 +1318,10 @@ def handle_limits_values(self, item, value, limits_set, ignore_persistence): item.limits.persistence_count = 0 def apply_format_string_and_units(self, item, value, value_type): + # Return None as-is - can't format a value that doesn't exist + if value is None: + return None + if value_type == "FORMATTED" or value_type == "WITH_UNITS": if item.format_string and value is not None: value = f"{item.format_string}" % value diff --git a/openc3/python/test/packets/parsers/test_limits_parser.py b/openc3/python/test/packets/parsers/test_limits_parser.py index 80b4f5b102..52f5413f7e 100644 --- a/openc3/python/test/packets/parsers/test_limits_parser.py +++ b/openc3/python/test/packets/parsers/test_limits_parser.py @@ -1,4 +1,4 @@ -# Copyright 2023 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -316,7 +316,7 @@ def test_takes_4_limits_values(self): self.pc.process_file(tf.name, "TGT1") item = self.pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] self.assertIsNotNone(item.limits.values["DEFAULT"]) - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04\x00" self.pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") self.pc.telemetry["TGT1"]["PKT1"].check_limits() self.assertEqual(item.limits.state, "GREEN") @@ -332,7 +332,7 @@ def test_takes_6_limits_values(self): self.pc.process_file(tf.name, "TGT1") item = self.pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] self.assertIsNotNone(item.limits.values["DEFAULT"]) - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04\x00" self.pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") self.pc.telemetry["TGT1"]["PKT1"].check_limits() self.assertEqual(item.limits.state, "BLUE") diff --git a/openc3/python/test/packets/test_commands.py b/openc3/python/test/packets/test_commands.py index 136a682e4d..8c29f25e94 100644 --- a/openc3/python/test/packets/test_commands.py +++ b/openc3/python/test/packets/test_commands.py @@ -1,4 +1,4 @@ -# Copyright 2024 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -181,7 +181,7 @@ def test_identify_logs_an_invalid_sized_buffer1(self): self.assertEqual(pkt.read("item1"), 1) self.assertEqual(pkt.read("item2"), 2) self.assertEqual(pkt.read("item3"), 3) - self.assertEqual(pkt.read("item4"), 0) + self.assertIsNone(pkt.read("item4")) self.assertIn( "TGT1 PKT1 buffer () received with actual packet length of 3 but defined length of 4", stdout.getvalue(), diff --git a/openc3/python/test/packets/test_packet.py b/openc3/python/test/packets/test_packet.py index 6554c94d93..4216b91e79 100644 --- a/openc3/python/test/packets/test_packet.py +++ b/openc3/python/test/packets/test_packet.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -1989,7 +1989,7 @@ def test_obfuscates_array_string_items(self): i = p.get_item("TEST1") i.obfuscate = True p.update_obfuscated_items_cache(i) - p.buffer = b"TESTTESTEST" + p.buffer = b"TESTTESTTEST" p.obfuscate() self.assertEqual(p.buffer, b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00") diff --git a/openc3/python/test/packets/test_packet_config.py b/openc3/python/test/packets/test_packet_config.py index 1b21ce55a6..c18337b571 100644 --- a/openc3/python/test/packets/test_packet_config.py +++ b/openc3/python/test/packets/test_packet_config.py @@ -1,4 +1,4 @@ -# Copyright 2025 OpenC3, Inc. +# Copyright 2026 OpenC3, Inc. # All Rights Reserved. # # This program is free software; you can modify and/or redistribute it @@ -1011,7 +1011,7 @@ def test_parses_the_conversion(self): tf.write(" READ_CONVERSION openc3/test_real_conversion.py\n") tf.seek(0) self.pc.process_file(tf.name, "TGT1") - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01\x00" self.assertEqual(self.pc.telemetry["TGT1"]["PKT1"].read("ITEM1"), 2) tf.close() @@ -1033,7 +1033,7 @@ def test_performs_a_polynomial_conversion(self): tf.write(" POLY_READ_CONVERSION 5 2\n") tf.seek(0) self.pc.process_file(tf.name, "TGT1") - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01\x00" self.assertEqual(self.pc.telemetry["TGT1"]["PKT1"].read("ITEM1"), 7.0) tf.close() @@ -1055,9 +1055,9 @@ def test_performs_a_segmented_polynomial_conversion(self): tf.write(" SEG_POLY_READ_CONVERSION 5 2 3\n") tf.seek(0) self.pc.process_file(tf.name, "TGT1") - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x01\x00" self.assertEqual(self.pc.telemetry["TGT1"]["PKT1"].read("ITEM1"), 3.0) - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x05" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x05\x00" self.assertEqual(self.pc.telemetry["TGT1"]["PKT1"].read("ITEM1"), 17.0) tf.close() @@ -1141,7 +1141,7 @@ def test_ensures_limits_sets_have_unique_names(self): item = self.pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] self.assertEqual(len(item.limits.values), 2) # Verify the last defined DEFAULT limits wins - self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04" + self.pc.telemetry["TGT1"]["PKT1"].buffer = b"\x04\x00" self.pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") self.pc.telemetry["TGT1"]["PKT1"].check_limits() self.assertEqual(item.limits.state, "RED_LOW") diff --git a/openc3/spec/packets/commands_spec.rb b/openc3/spec/packets/commands_spec.rb index 9c3b9a19f2..eeda324c09 100644 --- a/openc3/spec/packets/commands_spec.rb +++ b/openc3/spec/packets/commands_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -213,7 +213,7 @@ module OpenC3 expect(pkt.item1).to eql 1 expect(pkt.item2).to eql 2 expect(pkt.item3).to eql 3 - expect(pkt.item4).to eql 0 + expect(pkt.item4).to be_nil expect(stdout.string).to match(/TGT1 PKT1 received with actual packet length of 3 but defined length of 4/) end end diff --git a/openc3/spec/packets/packet_config_spec.rb b/openc3/spec/packets/packet_config_spec.rb index 05886effd3..4afbfaba00 100644 --- a/openc3/spec/packets/packet_config_spec.rb +++ b/openc3/spec/packets/packet_config_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -872,7 +872,7 @@ def call(packet) tf.puts ' READ_CONVERSION conversion2.rb' tf.close @pc.process_file(tf.path, "TGT1") - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01\x00" expect(@pc.telemetry["TGT1"]["PKT1"].read("ITEM1")).to eql 2 tf.unlink @@ -898,7 +898,7 @@ def call(packet) tf.puts ' POLY_READ_CONVERSION 5 2' tf.close @pc.process_file(tf.path, "TGT1") - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01\x00" expect(@pc.telemetry["TGT1"]["PKT1"].read("ITEM1")).to eql 7.0 tf.unlink @@ -923,9 +923,9 @@ def call(packet) tf.puts ' SEG_POLY_READ_CONVERSION 5 2 3' tf.close @pc.process_file(tf.path, "TGT1") - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x01\x00" expect(@pc.telemetry["TGT1"]["PKT1"].read("ITEM1")).to eql 3.0 - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x05" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x05\x00" expect(@pc.telemetry["TGT1"]["PKT1"].read("ITEM1")).to eql 17.0 tf.unlink @@ -1002,7 +1002,7 @@ def call(packet) item = @pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] expect(item.limits.values.length).to eql 2 # Verify the last defined DEFAULT limits wins - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04\x00" @pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") @pc.telemetry["TGT1"]["PKT1"].check_limits expect(item.limits.state).to eql :RED_LOW diff --git a/openc3/spec/packets/packet_spec.rb b/openc3/spec/packets/packet_spec.rb index 7307526101..569d375f08 100644 --- a/openc3/spec/packets/packet_spec.rb +++ b/openc3/spec/packets/packet_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # # Modified by OpenC3, Inc. -# All changes Copyright 2025, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -594,15 +594,15 @@ module OpenC3 expect(@p.read_item(i, :WITH_UNITS, "\x00\x01")).to eql ["0x0 V", "TRUE"] expect(@p.read("ITEM", :WITH_UNITS, "\x02\x03")).to eql ["FALSE", "0x3 V"] expect(@p.read_item(i, :WITH_UNITS, "\x02\x03")).to eql ["FALSE", "0x3 V"] - expect(@p.read("ITEM", :WITH_UNITS, "\x04")).to eql ["0x4 V"] - expect(@p.read_item(i, :WITH_UNITS, "\x04")).to eql ["0x4 V"] - expect(@p.read("ITEM", :WITH_UNITS, "\x04")).to eql ["0x4 V"] - expect(@p.read_item(i, :WITH_UNITS, "\x04")).to eql ["0x4 V"] + expect(@p.read("ITEM", :WITH_UNITS, "\x04\x00")).to eql ["0x4 V", "0x0 V"] + expect(@p.read_item(i, :WITH_UNITS, "\x04\x00")).to eql ["0x4 V", "0x0 V"] + expect(@p.read("ITEM", :WITH_UNITS, "\x04\x00")).to eql ["0x4 V", "0x0 V"] + expect(@p.read_item(i, :WITH_UNITS, "\x04\x00")).to eql ["0x4 V", "0x0 V"] i.read_conversion = GenericConversion.new("value / 2") expect(@p.read("ITEM", :WITH_UNITS, "\x02\x04")).to eql ["TRUE", "FALSE"] expect(@p.read_item(i, :WITH_UNITS, "\x02\x04")).to eql ["TRUE", "FALSE"] - expect(@p.read("ITEM", :WITH_UNITS, "\x08")).to eql ["0x4 V"] - expect(@p.read_item(i, :WITH_UNITS, "\x08")).to eql ["0x4 V"] + expect(@p.read("ITEM", :WITH_UNITS, "\x08\x00")).to eql ["0x4 V", "0x0 V"] + expect(@p.read_item(i, :WITH_UNITS, "\x08\x00")).to eql ["0x4 V", "0x0 V"] @p.define_item("item2", 0, 0, :DERIVED) i = @p.get_item("ITEM2") i.units = "V" diff --git a/openc3/spec/packets/parsers/limits_parser_spec.rb b/openc3/spec/packets/parsers/limits_parser_spec.rb index efbe1865ac..6b563b605c 100644 --- a/openc3/spec/packets/parsers/limits_parser_spec.rb +++ b/openc3/spec/packets/parsers/limits_parser_spec.rb @@ -14,7 +14,7 @@ # GNU Affero General Public License for more details. # Modified by OpenC3, Inc. -# All changes Copyright 2022, OpenC3, Inc. +# All changes Copyright 2026, OpenC3, Inc. # All Rights Reserved # # This file may also be used under the terms of a commercial license @@ -261,7 +261,7 @@ module OpenC3 @pc.process_file(tf.path, "TGT1") item = @pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] expect(item.limits.values[:DEFAULT]).not_to be_nil - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04\x00" @pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") @pc.telemetry["TGT1"]["PKT1"].check_limits expect(item.limits.state).to eql :GREEN @@ -278,7 +278,7 @@ module OpenC3 @pc.process_file(tf.path, "TGT1") item = @pc.telemetry["TGT1"]["PKT1"].items["ITEM1"] expect(item.limits.values[:DEFAULT]).not_to be_nil - @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04" + @pc.telemetry["TGT1"]["PKT1"].buffer = "\x04\x00" @pc.telemetry["TGT1"]["PKT1"].enable_limits("ITEM1") @pc.telemetry["TGT1"]["PKT1"].check_limits expect(item.limits.state).to eql :BLUE From 9eee8d8b93b08cff693c2d4b40b4bc254515a640 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Tue, 13 Jan 2026 20:43:39 -0700 Subject: [PATCH 4/6] Enable partial dependabot --- .github/dependabot.yml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 1e24735af9..a2e25c476f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,12 +1,11 @@ -# Dependabot configuration file for enabling PRs -# We have chosen to disable this as we manually update depen -# version: 2 -# updates: -# # Maintain dependencies for GitHub Actions -# - package-ecosystem: "github-actions" -# directory: "/" -# schedule: -# interval: "weekly" +version: 2 +updates: + # Maintain dependencies for GitHub Actions + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" +# Ignore all npm updates as we have a process for uding them with package_audit.rb # - package-ecosystem: "npm" # directories: # Location of package manifests # - "/openc3-cosmos-init/plugins/*" From be17fb431df170bcccac75ea7d93c01390e83768 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 14 Jan 2026 20:14:28 -0700 Subject: [PATCH 5/6] Fix python trivy issue --- openc3-ruby/Dockerfile | 1 + openc3/python/poetry.lock | 14 +++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/openc3-ruby/Dockerfile b/openc3-ruby/Dockerfile index 58396ed144..8e8bfd98c6 100644 --- a/openc3-ruby/Dockerfile +++ b/openc3-ruby/Dockerfile @@ -93,6 +93,7 @@ RUN apk update \ && pip3 install --upgrade pip setuptools \ && pip3 install poetry \ && pip3 install poetry-plugin-export \ + && pip3 install "jaraco.context>=6.1.0" \ # Note this is needed to prevent pip disappearing in some build environments && cp /openc3/venv/bin/pip* /openc3 diff --git a/openc3/python/poetry.lock b/openc3/python/poetry.lock index e9ea60685f..93f1ae7028 100644 --- a/openc3/python/poetry.lock +++ b/openc3/python/poetry.lock @@ -83,18 +83,18 @@ uvloop = ["uvloop (>=0.15.2)"] [[package]] name = "boto3" -version = "1.42.26" +version = "1.42.28" description = "The AWS SDK for Python" optional = false python-versions = ">=3.9" groups = ["main"] files = [ - {file = "boto3-1.42.26-py3-none-any.whl", hash = "sha256:f116cfbe7408e0a9153da363f134d2f1b5008f17ee86af104f0ce59a62be1833"}, - {file = "boto3-1.42.26.tar.gz", hash = "sha256:0fbcf1922e62d180f3644bc1139425821b38d93c1e6ec27409325d2ae86131aa"}, + {file = "boto3-1.42.28-py3-none-any.whl", hash = "sha256:7994bc2a094c1894f6a4221a1696c5d18af6c9c888191051866f1d05c4fba431"}, + {file = "boto3-1.42.28.tar.gz", hash = "sha256:7d56c298b8d98f5e9b04cf5d6627f68e7792e25614533aef17f815681b5e1096"}, ] [package.dependencies] -botocore = ">=1.42.26,<1.43.0" +botocore = ">=1.42.28,<1.43.0" jmespath = ">=0.7.1,<2.0.0" s3transfer = ">=0.16.0,<0.17.0" @@ -103,14 +103,14 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.42.26" +version = "1.42.28" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.9" groups = ["main"] files = [ - {file = "botocore-1.42.26-py3-none-any.whl", hash = "sha256:71171c2d09ac07739f4efce398b15a4a8bc8769c17fb3bc99625e43ed11ad8b7"}, - {file = "botocore-1.42.26.tar.gz", hash = "sha256:1c8855e3e811f015d930ccfe8751d4be295aae0562133d14b6f0b247cd6fd8d3"}, + {file = "botocore-1.42.28-py3-none-any.whl", hash = "sha256:d26c7a0851489ce1a18279f9802fe434bd736ea861d4888cc2c7d83fb1f6af8f"}, + {file = "botocore-1.42.28.tar.gz", hash = "sha256:0c15e78d1accf97df691083331f682e97b1bef73ef12dcdaadcf652abf9c182c"}, ] [package.dependencies] From bcf232b10088fba40e40b7ea39e846a170f69e10 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 14 Jan 2026 20:48:22 -0700 Subject: [PATCH 6/6] Remove pip install --- openc3-ruby/Dockerfile | 1 - 1 file changed, 1 deletion(-) diff --git a/openc3-ruby/Dockerfile b/openc3-ruby/Dockerfile index 8e8bfd98c6..58396ed144 100644 --- a/openc3-ruby/Dockerfile +++ b/openc3-ruby/Dockerfile @@ -93,7 +93,6 @@ RUN apk update \ && pip3 install --upgrade pip setuptools \ && pip3 install poetry \ && pip3 install poetry-plugin-export \ - && pip3 install "jaraco.context>=6.1.0" \ # Note this is needed to prevent pip disappearing in some build environments && cp /openc3/venv/bin/pip* /openc3