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