Skip to content

Fix Base45 dropping trailing bytes on non-ASCII input#45

Merged
dhondta merged 1 commit into
dhondta:mainfrom
gaoflow:fix-base45-multibyte-dataloss
Jul 5, 2026
Merged

Fix Base45 dropping trailing bytes on non-ASCII input#45
dhondta merged 1 commit into
dhondta:mainfrom
gaoflow:fix-base45-multibyte-dataloss

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Problem

The Base45 codec silently drops trailing bytes when the input contains non-ASCII content, producing output that is too short and no longer round-trips.

>>> import codext
>>> codext.encode(b'\xcf\xb1\x1b', 'base45')
b'OBQ'          # should be b'OBQR0'
>>> codext.decode(codext.encode(b'\xcf\xb1\x1b', 'base45'), 'base45')
b'\xcf\xb1'      # the trailing 0x1b byte is lost

Cause

base45_encode/base45_decode iterate with range(0, len(text), step) but index into t = b(text). Because the codec layer converts a bytes input to str (UTF-8) before the codec runs, b(text) is longer than text whenever the content is non-ASCII, so len(text) ends the loop early and the final group is never emitted. For b'\xcf\xb1\x1b' (\xcf\xb1 decodes to the single character U+03F1), text has length 2 while the byte sequence has length 3, dropping the third byte.

Fix

Iterate over len(t) (the actual byte sequence) in both functions. Encoded output now matches RFC 9285 and the reference base45 implementation, and encoding round-trips for arbitrary byte input.

Tests

Added test_codec_base45 covering the RFC 9285 vectors (AB, Hello!!, base-45), the exact regression value (b'\xcf\xb1\x1b' -> b'OBQR0'), and round-trips for binary inputs. The full test suite passes.

I worked on this with AI assistance under my direction and reviewed the change myself.

The Base45 encoder and decoder iterated with range(0, len(text), step)
while indexing into t = b(text). codext converts a bytes input to str
(UTF-8) before the codec runs, so for any non-ASCII content b(text) is
longer than text and len(text) stops the loop early, silently dropping
the trailing byte(s). For example encode(b'\xcf\xb1\x1b') returned
'OBQ' instead of 'OBQR0' and the value no longer round-tripped.

Iterate over len(t) (the actual byte sequence) instead. Output now
matches RFC 9285 and the reference base45 implementation, and encoding
round-trips for arbitrary byte input.
@dhondta dhondta requested a review from Copilot July 5, 2026 16:14
@dhondta dhondta self-assigned this Jul 5, 2026
@dhondta dhondta added the failure Something does not work as expected label Jul 5, 2026
@mergify

mergify Bot commented Jul 5, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Base45 encoding/decoding correctness bug where trailing bytes could be dropped when the (string) input contains non-ASCII characters, by iterating over the actual byte sequence length. Adds regression + RFC 9285 vector coverage to ensure byte inputs round-trip.

Changes:

  • Update Base45 encode/decode loops to iterate over len(t) (the byte sequence) instead of len(text) (the str length).
  • Add RFC 9285 test vectors and a regression test for b"\xcf\xb1\x1b" -> b"OBQR0".
  • Add several byte round-trip assertions for Base45.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_base.py Adds Base45 RFC vectors + regression + round-trip tests.
src/codext/base/base45.py Fixes iteration length to prevent silently dropping trailing bytes on non-ASCII input.

@dhondta dhondta merged commit 41a0866 into dhondta:main Jul 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

failure Something does not work as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants