Skip to content

fix(corekit,utilities): unquote a StringField's bytes on Python 3.8 through compat.unquote (#1693) - #1707

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1693-stringfield-unquote-py38
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1693-stringfield-unquote-py38

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort) — run on the changed files; no new findings

  • make test passes, and a test case covers the change — tests added; touched and related modules run, not the whole suite

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • 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

StringField(unquote=True) passed its raw octets to urllib.parse.unquote, which accepts bytes only from 3.9. On 3.8, every unquoting read raised TypeError: Expected str, got bytes. No schema in pcapkit sets unquote=True, so this hits user-built fields and the field tests.

  • Fix: pcapkit.utilities.compat exports unquote behind a sys.version_info gate. From 3.9 it is urllib.parse.unquote itself; below 3.9 it does what 3.9 does for bytes, unquote_to_bytes(value).decode(encoding, errors), and passes anything else to the interpreter's own unquote. StringField imports it for both call sites, including the UnicodeError fallback. The docs' compatibility table lists it.
  • Same behaviour as round 2's helper: on 3.9.25, 3.10.21 and 3.14.8, urllib.parse.unquote and the helper agree on all 20,142 / 20,142 / 20,318 cases (every encodings.aliases codec × 8 error handlers × 22 byte strings, plus str, bytearray, memoryview, a bytes subclass, int, None), results and exception type, message and UnicodeError attributes alike. The pre-3.9 body is the helper's, so round 2's 882,855-record differential still describes it (not re-run).
  • Tests: test_compat checks the running interpreter's unquote (urllib's own from 3.9) and the pre-3.9 branch, loaded under the faked (3, 5), against one 16-row table pinned to 3.9's results (bytes, str, bytearray, memoryview, defaults, both UnicodeDecodeError messages), the branch once more with a bytes-rejecting unquote. WARM_BEFORE_FAKING gains urllib.parse. The StringField table stays, plus a check that the field unquotes through compat.
  • Proof: test_compat, the touched module and test_field_roundtrip_unit.py pass under pytest and plain unittest on 3.8.20, 3.9.25, 3.10.21 and 3.14.8. Dropping the fallback's bytes branch fails test_python35_fallback_implementations on 3.14 and five tests on 3.8; main's strings.py fails test_unquote_is_the_compat_one on 3.14. On 3.14, test_compat plus the tests/corekit/ field tests cover every statement and branch of strings.py and compat.py. A line trace shows the gate's statements all run on 3.9, 3.10 and 3.14; on 3.8 all but the 3.9+ import.
  • Not run: 3.6, which has no interpreter here, and 3.7, which has no dependency set here that imports pcapkit.

Closes #1693

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 14, 2026
@JarryShaw

JarryShaw commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 6246836a2. A Sonnet cross-review checked two rounds. Round 1 (dca5b7973) was NEEDS CHANGES: the non-bytes branch at strings.py:53 had no test, and the PR body under-reported the interpreters tried. Both are fixed here. strings.py now has 0 missed statements and 0 partial branches from tests/corekit/ on 3.14, and a mutant deleting line 53 fails the new test on 3.8 and 3.14. str, bytearray and memoryview inputs behave exactly as on main. The four #1693 tests fail on base 3.8 and pass on this head on 3.8–3.14. An 882,855-record old-vs-new differential is identical on every version from 3.9 to 3.14. A pre-existing fallback leak found along the way is filed as #1706.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.42% (unit tier, Python 3.15, e37c14123, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18860 954 2368 868 91.06%
pcapkit/corekit 2372 67 762 35 96.49%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2887 144 1012 34 94.20%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17181 226 4590 190 98.07%
pcapkit/toolkit 1313 280 532 5 75.88%
pcapkit/utilities 508 4 154 4 98.79%
pcapkit/vendor 4417 2342 1010 157 43.38%

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

@JarryShaw

Copy link
Copy Markdown
Owner Author

Several notes:

  1. prefer Python version gates on the support. also, maybe should move _unquote as quote to utilities or corekit, under a shared common auxiliary helper method module.
  2. pcapkit/utilities/functools.rst no such module exists, the functions documented in it should go to their own target model Sphinx docs.
  3. pcapkit.protocols.schema.misc.byteorder module should be moved to either utilities or corekit under a shared common auxiliary helper method module
  4. pcapkit.utilities.chardet should be moved to corekit

@JarryShaw

Copy link
Copy Markdown
Owner Author

for 1, or easier, compat module with version gate branching would be a btter solution.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded:

  1. _unquote becomes a version-gated unquote in pcapkit/utilities/compat.py: urllib.parse.unquote itself on 3.9 and later, and the bytes-safe spelling below 3.9. strings.py imports it from there. A worker is on round 3; the verdict resets on the new head.
  2. functools.rst → docs(utilities): document the functions in functools.rst on their own modules' pages #1710.
  3. byteorder → refactor(corekit): move the byte-order field helpers out of protocols.schema.misc into a shared helper module #1711.
  4. chardet → refactor(corekit): move pcapkit.utilities.chardet into corekit #1712, blocked until this PR and fix(corekit): StringField's 'replace' decode fallback raises UnicodeError for codecs such as idna, on every Python #1706 merge, since they edit the same file.

@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
@JarryShaw JarryShaw added the review: needs-changes Cross-review at the current head says changes are required; see the verdict comment label Oct 14, 2026
@JarryShaw JarryShaw changed the title fix(corekit): unquote a StringField without passing bytes to urllib.parse.unquote (#1693) fix(corekit,utilities): unquote a StringField's bytes on Python 3.8 through compat.unquote (#1693) Oct 14, 2026
@JarryShaw
JarryShaw force-pushed the fix/1693-stringfield-unquote-py38 branch from 6246836 to 4a7f552 Compare October 14, 2026 03:38
@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 4a7f552e0. A Sonnet cross-review checked round 3.

  • The change: unquote now lives in pcapkit/utilities/compat.py behind sys.version_info < (3, 9), with no attribute probe. From 3.9 it is urllib.parse.unquote itself; below that it is the bytes-safe body. strings.py imports it from compat.
  • Unchanged behaviour: across 20,142–20,318 cases per interpreter, the stdlib unquote on 3.9+ gives the same result and the same error as round 2's helper, on bytes, str, bytearray and memoryview.
  • Tests: coverage on 3.14 is 100% for strings.py and compat.py. Mutants of each half fail the new tests on 3.8 and 3.14.
  • Subject corrected: the old one said bytes never reach urllib.parse.unquote, which is no longer true on 3.9+.
  • Heads-up for the merge: fix(utilities): fall back to the default threading excepthook before Python 3.10 (#1691) #1701 also edits compat.py's __all__ and test_compat.py's WARM_BEFORE_FAKING, so whichever merges second needs a small rebase.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

Rebasing onto 0b9c50882 now. The conflict is in pcapkit/utilities/compat.py and tests/utilities/test_compat.py, where #1701's excepthook gate and this PR's unquote gate both land. The rebase keeps both, and a Sonnet check follows before I push.

…hrough compat.unquote (#1693)

StringField(unquote=True) handed the raw field octets to
urllib.parse.unquote, which accepts bytes only from Python 3.9; on 3.8
every unquoting read raised "TypeError: Expected str, got bytes".

pcapkit.utilities.compat now exports unquote behind a sys.version_info
gate: from 3.9 it is urllib.parse.unquote itself; below 3.9 it spells
out what 3.9 does for bytes, unquote_to_bytes(value).decode(encoding,
errors), and passes anything else to the interpreter's own unquote.
StringField imports it for both call sites, including the UnicodeError
fallback, and the docs' compatibility table lists it.

Tests: test_compat checks the running interpreter's unquote and the
pre-3.9 branch (under the faked (3, 5), once more with a bytes-rejecting
unquote) against one table pinned to 3.9's results; WARM_BEFORE_FAKING
gains urllib.parse. The StringField table stays, plus a check that the
field unquotes through compat.
@JarryShaw

Copy link
Copy Markdown
Owner Author

resolve conflicts

@JarryShaw

Copy link
Copy Markdown
Owner Author

Conflicts resolved: rebased onto 27ccb5f1b as e37c14123.

Verdict: GOOD TO GO at e37c14123. A Sonnet cross-review replayed the merge itself, against Opus as author.

  • Resolution only: git conflicts in compat.py and test_compat.py alone. The resolution touches three hunks: the __all__ line, the line-number comment and WARM_BEFORE_FAKING. The strings.py, docs and string-field test patches are byte-identical to 4a7f552e0.
  • Both gates intact: fix(utilities): fall back to the default threading excepthook before Python 3.10 (#1691) #1701's excepthook gate (compat.py:267-272) is untouched, and unquote sits at :134-151. Both names are in __all__ once. Checked on 3.8.20, 3.9.25, 3.10.21 and 3.14.8.
  • No test lost: test_compat.py holds main's tests plus this PR's one, and the line numbers in the comment match the merged file.

@JarryShaw
JarryShaw force-pushed the fix/1693-stringfield-unquote-py38 branch from 4a7f552 to e37c141 Compare October 14, 2026 14:22
@JarryShaw
JarryShaw merged commit 4c750fe into main Oct 14, 2026
49 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 14, 2026
@JarryShaw
JarryShaw deleted the fix/1693-stringfield-unquote-py38 branch October 14, 2026 14:56
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(corekit): StringField passes bytes to urllib.parse.unquote, which accepts only str before Python 3.9

1 participant