Skip to content

fix(ftp): reject arguments that are not str, bytes or bytearray (#2410) - #2414

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2410-ftp-args-bytearray
Oct 28, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/2410-ftp-args-bytearray

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

What is the purpose of your pull request?

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

Fixes #2410.

FTP.make's args branch encoded anything that was not bytes, so FTP(cmmd='USER', args=bytearray(b'anonymous')) raised AttributeError: 'bytearray' object has no attribute 'encode'; args=42, args=True and args=memoryview(b'x') failed the same way. The gap predates #2365, which widened the sibling cmmd parameter and left args as it was.

The branch now takes a bytes or bytearray verbatim, encodes only a genuine str, and raises FTP: invalid arguments: must be a str, bytes or bytearray, not ... for anything else — the same shape as cmmd's guard from #2365. The args annotation gains bytearray, and the Args:/Raises: entries on make document it.

Fail-without-fix: with the guard reverted to elif isinstance(args, bytes), the new test fails 6/6 (5 subtests plus the bytearray acceptance), each with AttributeError: '<type>' object has no attribute 'encode' rather than the expected ProtocolError.

Tests: tests/protocols/ in full — 2021 passed / 51393 subtests / 0 failed, after regenerating examples/captures with examples/generators/make_samples.py. mypy reports nothing on ftp.py; pylint's findings on both touched files are unchanged from main.

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.
@JarryShaw JarryShaw added the bug Issues reporting a defect (set by the bug report template; a default, not an assessment) label Oct 28, 2026
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 28, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 28, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.5 milestone Oct 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 91.72% (unit tier, Python 3.15, fb7725f74, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19609 880 2620 867 92.12%
pcapkit/corekit 2596 74 818 37 96.51%
pcapkit/dumpkit 282 3 102 3 98.44%
pcapkit/foundation 3045 24 1062 26 98.78%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17578 195 4670 181 98.30%
pcapkit/toolkit 924 36 334 10 95.55%
pcapkit/utilities 502 7 144 5 98.14%
pcapkit/vendor 4449 1961 1020 167 52.64%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Review: GOOD TO GO at fb7725f7 (Sonnet). 38 argument types across 6 call shapes confirmed: every rejected type raises the exact message, bytearray builds correctly on all shapes, no regression on str/bytes/subclasses (228 records total). Fail-without-fix confirmed (6 failed on revert, all AttributeError). tests/protocols/ delta is exactly the new test (2021 vs main's 2020). mypy/pylint on ftp.py unchanged; the PR's claim that pylint is unchanged on the test file is slightly off (39→41, two new local-import findings matching the cmmd sibling's own style) — non-blocking, CI only lints pcapkit/.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 28, 2026
@JarryShaw
JarryShaw merged commit e0847a1 into main Oct 28, 2026
68 checks passed
@JarryShaw
JarryShaw deleted the fix/2410-ftp-args-bytearray branch October 28, 2026 15:37
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 28, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issues reporting a defect (set by the bug report template; a default, not an assessment) fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(application): FTP.make's args rejects bytearray/int with AttributeError instead of ProtocolError

1 participant