From 0603014fa153c47e5d00c105d92b64cde3267d24 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 26 Oct 2026 16:46:34 -0400 Subject: [PATCH] fix(ftp): reject a command that is not str or bytes with ProtocolError (#2290) FTP.make fell back to cmmd.value for anything not str or bytes. Command is a StrEnum, so a member never reached that branch, and every value that did (an int, a bool, a bytearray) raised a bare AttributeError. Accept a bytearray as bytes, and raise "FTP: invalid command: must be a str, bytes or bytearray, not ..." for anything else, a bool included, as #2256 does for ARP addresses. Test: the exact message for int, bool, float and a .value-carrying object, and a bytearray command building like bytes; the make test that pinned the duck-typed .value object now uses a bytearray. --- pcapkit/protocols/application/ftp.py | 19 ++++++++++------ tests/protocols/application/test_ftp_unit.py | 23 ++++++++++++++++++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/pcapkit/protocols/application/ftp.py b/pcapkit/protocols/application/ftp.py index 8f608ad342..2a8df6fc22 100644 --- a/pcapkit/protocols/application/ftp.py +++ b/pcapkit/protocols/application/ftp.py @@ -166,7 +166,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_FTP': return ftp def make(self, - cmmd: 'Optional[Enum_Command | str | bytes]' = None, + cmmd: 'Optional[Enum_Command | str | bytes | bytearray]' = None, code: 'Optional[Enum_ReturnCode | int | str | bytes]' = None, args: 'Optional[str | bytes]' = None, more: 'bool' = False, @@ -186,7 +186,9 @@ def make(self, which keeps the case the command was read in. Args: - cmmd: FTP command. + cmmd: FTP command, a :class:`str` (an + :class:`~pcapkit.const.ftp.command.Command` member is one), + :class:`bytes` or :class:`bytearray`. code: FTP status code. args: Optional FTP command arguments and/or status messages. more: More status messages to follow for response packets. @@ -202,17 +204,20 @@ def make(self, Raises: ProtocolError: If neither or both of ``cmmd`` and ``code`` are - given, or if ``lines`` are given other than as the rest of a - multi-line reply. + given, if ``cmmd`` is not a :class:`str`, :class:`bytes` or + :class:`bytearray` (a :obj:`bool` included), or if ``lines`` + are given other than as the rest of a multi-line reply. """ if cmmd is not None and code is None: - if isinstance(cmmd, bytes): - prefix = cmmd + # NOTE: ``Command`` is a ``StrEnum``, so a member takes the ``str`` + # branch; anything else is the wrong type (#2290). + if isinstance(cmmd, (bytes, bytearray)): + prefix = bytes(cmmd) elif isinstance(cmmd, str): prefix = cmmd.encode() else: - prefix = cmmd.value + raise ProtocolError(f'FTP: invalid command: must be a str, bytes or bytearray, not {cmmd!r}') if raw_cmmd is not None and raw_cmmd.upper() == prefix.upper(): prefix = raw_cmmd diff --git a/tests/protocols/application/test_ftp_unit.py b/tests/protocols/application/test_ftp_unit.py index c72f042c3c..37dee6535e 100644 --- a/tests/protocols/application/test_ftp_unit.py +++ b/tests/protocols/application/test_ftp_unit.py @@ -173,7 +173,7 @@ def test_ftp_make_builds_request_and_response_packets(self) -> None: ftp = object.__new__(FTP) request = ftp.make(cmmd=Command.USER, args='anonymous') request_bytes = ftp.make(cmmd=b'NOOP') - request_value = ftp.make(cmmd=SimpleNamespace(value=b'PWD')) + request_bytearray = ftp.make(cmmd=bytearray(b'PWD')) response = ftp.make(code=ReturnCode.CODE_220, args='Ready', more=True) response_bytes = ftp.make(code=b'230', args=b'Logged in') @@ -181,7 +181,7 @@ def test_ftp_make_builds_request_and_response_packets(self) -> None: # separator is written without arguments, and none after a hyphen. self.assertEqual(request.data, b'USER anonymous\r\n') self.assertEqual(request_bytes.data, b'NOOP\r\n') - self.assertEqual(request_value.data, b'PWD\r\n') + self.assertEqual(request_bytearray.data, b'PWD\r\n') self.assertEqual(response.data, b'220-Ready\r\n') self.assertEqual(response_bytes.data, b'230 Logged in\r\n') @@ -196,6 +196,25 @@ def test_ftp_make_rejects_ambiguous_packet_type(self) -> None: with self.assertRaises(ProtocolError): ftp.make(cmmd='USER', code=220) + def test_ftp_make_rejects_a_command_that_is_not_str_bytes_or_bytearray(self) -> None: + """A command of any other type raises ``ProtocolError`` (#2290).""" + from pcapkit.protocols.application.ftp import FTP + from pcapkit.utilities.exceptions import ProtocolError + + ftp = object.__new__(FTP) + + for cmmd in (42, True, 1.5, SimpleNamespace(value=b'PWD')): + with self.subTest(cmmd=cmmd): + with self.assertRaises(ProtocolError) as context: + ftp.make(cmmd=cmmd, args='anon') + self.assertEqual( + str(context.exception), + f'FTP: invalid command: must be a str, bytes or bytearray, not {cmmd!r}') + + # a bytearray is accepted as bytes are, and keeps the captured case + self.assertEqual(ftp.make(cmmd=bytearray(b'user'), args='anon', raw_cmmd=b'user').data, + b'user anon\r\n') + def test_ftp_make_data_preserves_request_and_response_fields(self) -> None: from pcapkit.const.ftp.command import Command from pcapkit.const.ftp.return_code import ReturnCode