Issue #368: Bad packet size handling in built-in server handlers #456
Conversation
| raise ValueError(msg) | ||
|
|
||
| self._pkt_defn = tlm_dict[self.packet] | ||
| self._pkt_defn = self.tlm_dict[self.packet_type] |
There was a problem hiding this comment.
@JHofman728 Since you are already grabbing the packet definition here, there really should be no need for the new get_packet_lengths() function below.
|
|
||
| if not self.packet: | ||
| msg = 'PacketHandler: No packet name provided in handler config as key "packet"' | ||
| if not self.packet_type: |
There was a problem hiding this comment.
Could you please update this field to be packet_name?
nttoole
changed the title
| return pickle.dumps((self._pkt_defn.uid, input_data), 2) | ||
|
|
||
| if self._pkt_defn.nbytes != packet.nbytes: | ||
| msg = f"PacketHandler: Packet length of packet does not match packet definition." |
There was a problem hiding this comment.
Could you please rephrase to: "PacketHandler: Packet data length does not match packet definition"
|
@JimHofman Were you able to successfully run pytest on this branch? |
|
|
||
| if self._pkt_defn.nbytes != packet.nbytes: | ||
| msg = f"PacketHandler: Packet data length does not match packet definition." | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
@JimHofman Could this be an error log message instead of thrown error (same as CCSDS handler)
There was a problem hiding this comment.
Done, will work on pickle error before pushing.
There was a problem hiding this comment.
Looks like the pickle error is coming from the unit tests that test bad packet length. It's is set up by passing a 1552 packet to an Ethernet handler, so it makes sense.
There was a problem hiding this comment.
This check doesn't even look valid? You should be checking the length of the received input data not the number of bytes in a packet definition.
Add error handling for PacketHandler and CCSDSPacketHandler for check on the size relative to the configured packet definitions.
Fixes #368