From fb7725f74cae23eb0a9d0767bf35acf165f767a0 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 28 Oct 2026 10:25:41 -0400 Subject: [PATCH] fix(ftp): reject arguments that are not str, bytes or bytearray (#2410) FTP.make's args branch encoded anything that was not bytes, so a bytearray, int, bool or memoryview raised a bare AttributeError ('bytearray' object has no attribute 'encode') instead of a ProtocolError. The gap predates #2365, which widened the sibling cmmd parameter but left args as it was. Accept a bytes or bytearray verbatim, encode only a genuine str, and raise "FTP: invalid arguments: must be a str, bytes or bytearray, not ..." for anything else, mirroring cmmd's guard from #2365. Widened the args annotation to include bytearray and extended the Args:/Raises: docstring entries. Test: tests/protocols/application/test_ftp_unit.py gains test_ftp_make_rejects_arguments_that_are_not_str_bytes_or_bytearray, pinning the exact message for int, bool, float, memoryview and a .value-carrying object, and a bytearray argument building on both a request and a reply. Confirmed fail-without-fix: with the guard reverted the test fails 6/6 with AttributeError rather than ProtocolError. tests/protocols/ in full: 2021 passed / 51393 subtests / 0 failed, after regenerating examples/captures via examples/generators/make_samples.py. mypy reports nothing on ftp.py and pylint's findings on both touched files are unchanged from main. --- pcapkit/protocols/application/ftp.py | 24 +++++++++++++------- tests/protocols/application/test_ftp_unit.py | 21 +++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pcapkit/protocols/application/ftp.py b/pcapkit/protocols/application/ftp.py index a4a801032e..37e42b1c42 100644 --- a/pcapkit/protocols/application/ftp.py +++ b/pcapkit/protocols/application/ftp.py @@ -174,7 +174,7 @@ def read(self, length: 'Optional[int]' = None, **kwargs: 'Any') -> 'Data_FTP': def make(self, cmmd: 'Optional[Enum_Command | str | bytes | bytearray]' = None, code: 'Optional[Enum_ReturnCode | int | str | bytes]' = None, - args: 'Optional[str | bytes]' = None, + args: 'Optional[str | bytes | bytearray]' = None, more: 'bool' = False, charset: 'Optional[str]' = None, raw_cmmd: 'Optional[bytes]' = None, @@ -196,7 +196,8 @@ def make(self, :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. + args: Optional FTP command arguments and/or status messages, a + :class:`str`, :class:`bytes` or :class:`bytearray`. more: More status messages to follow for response packets. charset: Encoding for :obj:`str` arguments, **UTF-8** if not given. raw_cmmd: FTP command as received, in its original case. @@ -210,9 +211,10 @@ def make(self, Raises: ProtocolError: If neither or both of ``cmmd`` and ``code`` are - 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. + given, if ``cmmd`` or ``args`` 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: @@ -240,12 +242,18 @@ def make(self, if lines and not (mf and self._test_lines(prefix, lines)): raise ProtocolError('FTP: lines must close a multi-line reply') + # NOTE: bytes and a bytearray are written verbatim and only a ``str`` is + # encoded; anything else is the wrong type, as it is for ``cmmd`` above + # (#2410). if args is None: suffix = b'' - elif isinstance(args, bytes): - suffix = args - else: + elif isinstance(args, (bytes, bytearray)): + suffix = bytes(args) + elif isinstance(args, str): suffix = args.encode(charset or 'utf-8') + else: + raise ProtocolError('FTP: invalid arguments: ' + f'must be a str, bytes or bytearray, not {args!r}') # the separating SP, absent with no arguments and after a hyphen sep = b'' if args is None or mf else b' ' diff --git a/tests/protocols/application/test_ftp_unit.py b/tests/protocols/application/test_ftp_unit.py index eb20d758ef..f784923f3b 100644 --- a/tests/protocols/application/test_ftp_unit.py +++ b/tests/protocols/application/test_ftp_unit.py @@ -215,6 +215,27 @@ def test_ftp_make_rejects_a_command_that_is_not_str_bytes_or_bytearray(self) -> self.assertEqual(ftp.make(cmmd=bytearray(b'user'), args='anon', raw_cmmd=b'user').data, b'user anon\r\n') + def test_ftp_make_rejects_arguments_that_are_not_str_bytes_or_bytearray(self) -> None: + """Arguments of any other type raise ``ProtocolError`` (#2410).""" + from pcapkit.protocols.application.ftp import FTP + from pcapkit.utilities.exceptions import ProtocolError + + ftp = object.__new__(FTP) + + for args in (42, True, 1.5, memoryview(b'anon'), SimpleNamespace(value=b'anon')): + with self.subTest(args=args): + with self.assertRaises(ProtocolError) as context: + ftp.make(cmmd='USER', args=args) + self.assertEqual( + str(context.exception), + f'FTP: invalid arguments: must be a str, bytes or bytearray, not {args!r}') + + # a bytearray is written verbatim, as bytes are, on a request and a reply + self.assertEqual(ftp.make(cmmd='USER', args=bytearray(b'anonymous')).data, + b'USER anonymous\r\n') + self.assertEqual(ftp.make(code=230, args=bytearray(b'Logged in')).data, + b'230 Logged in\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