Skip to content

fix(corekit): decode as UTF-8 when a StringField's charset rejects 'replace' (#1706) - #1728

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1706-stringfield-replace-fallback
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1706-stringfield-replace-fallback

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

Please follow the guide below

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

Closes #1706.

  • Bug: StringField retried a failed decode with errors='replace' outside any try. A codec that also raises under 'replace' therefore let a UnicodeError escape unpack. That is idna on every Python, undefined, and punycode on non-ASCII octets before 3.13.
  • Fix: one more fallback, to UTF-8 with 'replace', which cannot raise. UTF-8 is the fallback encoding the docstring already names, and the one the unquote branch already uses. The existing lossless check returns a DecodedString, so packing gives back the captured octets byte for byte. The errors docstring no longer says such a codec still raises.
  • Trade-off: with errors='ignore', idna on b'xn--caf-dma' now reads as 'xn--caf-dma', while 'strict' still gives 'café'.
  • Tests:
    • New test: test_a_codec_that_rejects_replace_does_not_raise fails on main (4 subtests on 3.14, 7 errors on 3.8) and passes on 3.8, 3.11, 3.14 and 3.15.
    • Codec sweep: a reviewer ran every codec in encodings (111 or 112) under five error schemes, over 165 byte samples, on 3.8, 3.11 and 3.14. That found 0 exceptions and 0 round-trip mismatches.

…eplace' (#1706)

- StringField.post_process retried a failed decode with 'replace' outside
  any try, so a codec that raises under 'replace' too let UnicodeError
  escape unpack: idna (strict only) on every Python, and punycode on
  non-ASCII octets before 3.13.
- The retry now falls back once more, to UTF-8 with 'replace', which
  cannot raise; the existing lossless check returns a DecodedString, so
  the field packs back the captured octets.
- The errors docstring no longer says such a codec still raises.

Tests: TestStringFieldLossless.test_a_codec_that_rejects_replace_does_not_raise
(4 subtests fail on origin/main on 3.14, 7 on 3.8); the lossless module
passes on 3.8, 3.11, 3.14 and 3.15.

Closes #1706
@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 moved this to In review in PyPCAPKit Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 7f028562f. A Sonnet cross-review checked it; Opus wrote it.

  • Codec sweep: every text codec in encodings was run under five error schemes, with unquote both on and off, over 165 byte samples, on 3.8, 3.11 and 3.14. That is about 198k cases per interpreter, with 0 exceptions and 0 round-trip mismatches.
  • Before the fix: the same sweep on main raised 1,697 UnicodeErrors on 3.14, from idna and undefined. On 3.8 punycode raised as well.
  • Fail before: the new test fails on main and passes on 3.8 and 3.14.
  • Scope: strings.py changes in two hunks, the docstring and the fallback. No import line changes, so it does not collide with refactor(corekit)!: move chardet into corekit (#1712) #1729.

@github-actions

Copy link
Copy Markdown
Contributor

Coverage: 89.35% (unit tier, Python 3.15, 7f028562f, 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 2375 67 762 35 96.49%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 2885 144 1010 34 94.20%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17181 226 4590 190 98.07%
pcapkit/toolkit 1148 290 450 10 71.21%
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
JarryShaw merged commit 59dd261 into main Oct 14, 2026
54 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/1706-stringfield-replace-fallback branch October 14, 2026 16:11
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 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's 'replace' decode fallback raises UnicodeError for codecs such as idna, on every Python

1 participant