FIX: preserve unknown characters in BrailleConverter - #2309
Conversation
Characters outside the Braille lookup table (e.g. '@', '%', '+', '<', '&',
accented letters, CJK) were silently dropped during conversion, so an
obfuscated prompt could reach the target model with a different meaning
than intended ("a@b.com" became "ab.com"). Every other converter in
this codebase passes unknown characters through unchanged.
Pass through characters that are neither escape characters, mapped
characters, nor mapped uppercase letters. The number-mode state machine
is unchanged: a passed-through non-digit still exits number mode.
Closes microsoft#2308
Signed-off-by: fei <204683769+feiiiiii5@users.noreply.github.com>
|
@microsoft-github-policy-service agree |
Review follow-ups on top of the pass-through fix:
- Map the 20 printable ASCII characters that had no Braille cell
(@ # % & * + < = > " [ ] \ ^ _ ` { } | ~) to their two-cell Unified English
Braille sequences, verified against liblouis' en-ueb-chardefs.uti. These were
the characters motivating the fix, so encoding them beats leaking raw ASCII
into the obfuscated output. Pass-through now only applies to genuinely
unmappable input (accented and CJK letters, emoji, control characters).
- Remove the escape_characters branch. With the pass-through fallback in place
it is equivalent to the fallback and only obscures intent.
- Document the pass-through contract in the class docstring instead of an
inline comment.
- Replace the two new tests with parametrized ones asserting exact output,
matching the existing punctuation-cell test. Adds coverage for escape
character preservation and for number mode resuming across a symbol.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 46d2071a-7f67-4855-84da-d0f19efb8a53
romanlutz
left a comment
There was a problem hiding this comment.
Thanks for catching this — the drop bug was real and the root-cause writeup was accurate.
I pushed a follow-up commit on top of yours rather than leaving it as review comments:
- Mapped the 20 remaining printable ASCII characters (
@ # % & * + < = > " [ ] \ ^ _{ } | ~) to their two-cell UEB sequences, verified against liblouis'en-ueb-chardefs.uti`. Those were exactly the characters motivating the fix, and for an obfuscation converter, encoding them beats leaking raw ASCII into the output. Pass-through is now the fallback only for genuinely unmappable input (accented/CJK letters, emoji, control characters). Every printable ASCII character now maps to a Braille cell. - Removed the
escape_charactersbranch. Once theelsefallback exists, that branch is equivalent to the fallback —\n,\r,\tfall through and are emitted verbatim either way (\vand\f, which were never in the list, already behaved identically). Dead control flow that would mislead the next reader. - Moved the pass-through contract into the class docstring, which previously still described the old lossy behavior.
- Rewrote the two new tests to assert exact output and parametrize, matching the
test_braille_converter_punctuation_cellspattern already in the file.assert "@" in outputwould pass on a duplicated or misplaced character. The new set also covers escape-character preservation (guarding the branch removal) and number mode resuming across a symbol ("1+2"→⠼⠁⠐⠖⠼⠃).
Two pre-existing table bugs are out of scope here, but worth a separate issue: "$" and "." both map to \u2832, and "("/")" both map to \u2836. UEB has $ = ⠈⠎ and distinct ⠐⠣/⠐⠜ for parentheses.
One note on the PR description: "Every other converter in this codebase passes unknown characters through unchanged" isn't quite right — MorseConverter emits a sentinel error_char = "........" for unmapped input. Doesn't change the conclusion that dropping was wrong.
Approving. Will merge once CI is green.
Root Cause
BrailleConverter._get_braileonly emits characters found in its Braille lookup table (or escape characters / mapped uppercase letters). Any other character —@,%,+,<,&, accented letters, CJK — is silently dropped, so an obfuscated prompt can reach the target model with a different meaning than intended:Every other converter in this codebase passes unknown characters through unchanged (
AtbashConverterusesstr.translate, which leaves unmapped characters as-is;ArabiziConverterandCharacterSpaceConverterkeep them), so this deviation is an inconsistency as well as a correctness bug.Fix
Pass through characters that are neither escape characters, mapped characters, nor mapped uppercase letters:
The number-mode state machine is unchanged: a passed-through non-digit still exits number mode (the existing per-character state reset line applies to every branch).
Test
Added to
tests/unit/converter/test_braille_converter.py:test_braille_converter_preserves_unknown_characters—@,+,%,é,中all survive conversion.test_braille_converter_known_characters_still_encoded— known characters are still converted to Braille (no over-correction).Full converter suite: 1072 passed, 27 skipped (skips are Azure Speech SDK / audio-related, unrelated).
ruff check/ruff format --check: clean.Diff scope
2 files, +18/-4:
pyrit/converter/braille_converter.py,tests/unit/converter/test_braille_converter.py.AI Disclosure
AI-assisted root-cause analysis, initial draft, and test scaffolding; human review of the conversion semantics and number-mode behavior.
Not PR-related failures
The 27 skipped tests require the Azure Speech SDK or audio tooling not present locally.