Skip to content

Commit

Permalink
Fix potential frame parsing issue where an empty payload is still valid
Browse files Browse the repository at this point in the history
  • Loading branch information
puddly committed Sep 26, 2020
1 parent 469f590 commit 431f5fe
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 9 deletions.
19 changes: 19 additions & 0 deletions tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,22 @@ def test_command_replace_partial():

assert command1.replace() == command1
assert command1.replace(SysId=0x13) == command2


def test_command_possibly_empty_payload():
class TestSubsystem(t.CommandsBase, subsystem=t.Subsystem.SYS):
Test = t.CommandDef(
t.CommandType.AREQ,
0x00,
rsp_schema=(t.Param("Data", t.Bytes, "Can be any length"),),
)

Test = TestSubsystem.Test.Callback

assert Test.from_frame(
frames.GeneralFrame(header=Test.header, data=b"test")
) == Test(Data=t.Bytes(b"test"))

assert Test.from_frame(frames.GeneralFrame(header=Test.header, data=b"")) == Test(
Data=t.Bytes(b"")
)
23 changes: 14 additions & 9 deletions zigpy_znp/types/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,16 +407,21 @@ def from_frame(cls, frame, *, ignore_unparsed=False) -> "CommandBase":
params = {}

for param in cls.schema:
if data:
try:
params[param.name], data = param.type.deserialize(data)
elif not param.optional:
# If we're out of data but the parameter is required, this is bad
raise ValueError(
f"Data has been consumed but required parameters remain: {param}"
)
else:
# If we're out of data and the parameter is optional, we're done
break
except ValueError:
if not data and param.optional:
# If we're out of data and the parameter is optional, we're done
break
elif not data and not param.optional:
# If we're out of data but the parameter is required, this is bad
raise ValueError(
f"Frame data is truncated (parsed {params}),"
f" required parameter remains: {param}"
)
else:
# Otherwise, let the exception happen
raise

if data:
msg = (
Expand Down

0 comments on commit 431f5fe

Please sign in to comment.