diff --git a/smp/image_management.py b/smp/image_management.py index 868c4c0..7a117d4 100644 --- a/smp/image_management.py +++ b/smp/image_management.py @@ -85,12 +85,30 @@ class ImageUploadWriteResponse(message.WriteResponse): This is the offset of the next byte to be written. If the offset is equal to the length of the image, the upload is complete. """ + match: bool | None = None """Indicates if the uploaded data successfully matches the provided SHA256. Only sent in the final packet if CONFIG_IMG_ENABLE_IMAGE_CHECK is enabled. """ + rc: int | None = None + """Legacy field that contains a return code; possibly `MGMT_ERR`. + + This field may be present on old SMP server implementations or new SMP + server implementations that have set + `CONFIG_MCUMGR_SMP_LEGACY_RC_BEHAVIOUR=y` for backwards compatibility with + old SMP clients. + + Note that we are not validating this field because we don't necessarily + trust the server to send us valid values. If this value is present, then it + indicates use of an SMP server that is out of spec and interpretation of the + value should be done with reference to that server's source code, rather + that the SMP specification. + + Zephyr source code reference: https://github.com/zephyrproject-rtos/zephyr/blob/91a1e706535b2f99433280513c5bc66dfb918506/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c#L397-L400 + """ # noqa: E501 + class ImageEraseRequest(message.WriteRequest): _GROUP_ID = header.GroupId.IMAGE_MANAGEMENT diff --git a/tests/test_image_management.py b/tests/test_image_management.py index 0ea9a24..fa37492 100644 --- a/tests/test_image_management.py +++ b/tests/test_image_management.py @@ -244,8 +244,7 @@ def test_ImageUploadWriteRequest() -> None: @pytest.mark.parametrize("off", [None, 0, 1, 0xFFFF, 0xFFFFFFFF]) @pytest.mark.parametrize("match", [None, True, False]) -@pytest.mark.parametrize("rc", [None, 0, 1, 10]) -def test_ImageUploadWriteResponse(off: int | None, match: bool | None, rc: int | None) -> None: +def test_ImageUploadWriteResponse(off: int | None, match: bool | None) -> None: assert_header = make_assert_header( smpheader.GroupId.IMAGE_MANAGEMENT, smpheader.OP.WRITE_RSP, @@ -280,3 +279,51 @@ def test_ImageUploadWriteResponse(off: int | None, match: bool | None, rc: int | assert_header(r) assert r.off == off assert r.match == match + + +@pytest.mark.parametrize("off", [None, 0, 1, 0xFFFF, 0xFFFFFFFF]) +@pytest.mark.parametrize("match", [None, True, False]) +@pytest.mark.parametrize("rc", [None, 0, 1, -23478934]) +def test_legacy_ImageUploadWriteResponse( + off: int | None, match: bool | None, rc: int | None +) -> None: + assert_header = make_assert_header( + smpheader.GroupId.IMAGE_MANAGEMENT, + smpheader.OP.WRITE_RSP, + smpheader.CommandId.ImageManagement.UPLOAD, + None, + ) + r = smpimg.ImageUploadWriteResponse(off=off, match=match, rc=rc) + + assert_header(r) + assert r.off == off + assert r.match == match + assert r.rc == rc + + r = smpimg.ImageUploadWriteResponse.loads(r.BYTES) + assert_header(r) + assert r.off == off + assert r.match == match + assert r.rc == rc + + if sys.version_info >= (3, 9): + cbor_dict = ( + {} + | ({"off": off} if off is not None else {}) + | ({"match": match} if match is not None else {}) + | ({"rc": rc} if rc is not None else {}) + ) + else: + cbor_dict = {} + if off is not None: + cbor_dict["off"] = off + if match is not None: + cbor_dict["match"] = match + if rc is not None: + cbor_dict["rc"] = rc + + r = smpimg.ImageUploadWriteResponse.load(r.header, cbor_dict) + assert_header(r) + assert r.off == off + assert r.match == match + assert r.rc == rc diff --git a/tests/test_regressions.py b/tests/test_regressions.py new file mode 100644 index 0000000..d42cb92 --- /dev/null +++ b/tests/test_regressions.py @@ -0,0 +1,34 @@ +"""This file is here to prevent specific regressions.""" + +from typing import Final + +import pytest + +import smp.header as smphdr +import smp.image_management as smpimg + + +def test_smpclient_41() -> None: + """https://github.com/intercreate/smpclient/issues/41""" + + RESPONSE: Final = b"\x03\x00\x00\x0c\x00\x01\x02\x01\xbfbrc\x00coff\x18\xa5\xff" + """A legacy `ImageUploadWriteResponse` that contains the rc field.""" + + r: Final = smpimg.ImageUploadWriteResponse.loads(RESPONSE) + + assert r.header.op == smphdr.OP.WRITE_RSP + assert r.header.version == smphdr.Version.V1 + assert r.header.flags == smphdr.Flag(0) + assert r.header.length == 12 + assert r.header.group_id == smphdr.GroupId.IMAGE_MANAGEMENT + assert r.header.sequence == 2 + assert r.header.command_id == smphdr.CommandId.ImageManagement.UPLOAD + + assert r.off == 165 + assert r.rc == 0 + + with pytest.raises(ValueError): + smpimg.ImageManagementErrorV1.loads(RESPONSE) + + with pytest.raises(ValueError): + smpimg.ImageManagementErrorV2.loads(RESPONSE)