Fix Code128 build() producing different output on repeated calls#264
Open
apoorvdarshan wants to merge 1 commit into
Open
Fix Code128 build() producing different output on repeated calls#264apoorvdarshan wants to merge 1 commit into
apoorvdarshan wants to merge 1 commit into
Conversation
Code128._build mutated self._charset and self._digit_buffer (both set only in __init__) but never reset them. After a build that switched to charset A/B or left a digit buffered, a subsequent build()/encoded call started from stale state and emitted a different, incorrect bit string. Reset both to their initial values at the start of _build() so repeated calls on the same instance are deterministic. Fixes WhyNotHugo#143
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #143
Root cause
Code128._buildmutatesself._charsetandself._digit_bufferwhile it walks the code (switching charsets via_new_charset, buffering digit pairs in charset C). Both attributes are initialised only once, in__init__. When a build ends in charset A/B — or with a digit still buffered — those attributes are left in a "dirty" state. The next call tobuild()/encodedstarts from that stale state instead of the intended_charset = "C"/ empty buffer, so it emits a different (and incorrect) bit string.Reproduction
Observed (before the fix) — the first call is correct, later calls diverge:
The divergence is the start code: the first build begins in charset C (
START_C= 105); after it the instance is left in charset B, so later builds begin withSTART_B= 104. A freshly constructedCode128("12A")always reproduces the first (correct) line, confirming the later output is the buggy one.After the fix every call returns the same, correct value:
The original report's all-digit example (
Code128("1170773")) never left charset C, so it happened to be stable on currentmain; inputs that switch charset (e.g."12A","123ABC456") expose the bug.Fix
Reset
self._charset = "C"andself._digit_buffer = ""at the start of_build(), so each build begins from the same initial state regardless of what the previous one left behind. This is the smallest localized change and touches only the entry of_build.Tests
Added
tests/test_code128.pywith three regression tests asserting that repeatedbuild()/encodedcalls on one instance are deterministic and that a reused instance matches a freshly constructed one. These tests fail on unmodifiedmainand pass with the fix. The full existing suite (52 tests, including the GS1-128 charset-C build tests) continues to pass, andruff check/ruff format --checkare clean.Disclosure: prepared with AI assistance; reviewed and verified locally.