Skip to content

FIX: preserve unknown characters in BrailleConverter - #2309

Merged
romanlutz merged 2 commits into
microsoft:mainfrom
feiiiiii5:fix/braille-converter-preserve-unknown-chars
Aug 2, 2026
Merged

FIX: preserve unknown characters in BrailleConverter#2309
romanlutz merged 2 commits into
microsoft:mainfrom
feiiiiii5:fix/braille-converter-preserve-unknown-chars

Conversation

@feiiiiii5

Copy link
Copy Markdown
Contributor

Root Cause

BrailleConverter._get_braile only 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:

"What's 2+2?"  →  "22?"      ("+" dropped)
"a@b.com"      →  "ab.com"   ("@" dropped)
"café 中文"    →  "caf "     ("é 中文" dropped)

Every other converter in this codebase passes unknown characters through unchanged (AtbashConverter uses str.translate, which leaves unmapped characters as-is; ArabiziConverter and CharacterSpaceConverter keep 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:

else:
    output += char

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.

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>
@feiiiiii5

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@romanlutz romanlutz self-assigned this Aug 2, 2026
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 romanlutz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_characters branch. Once the else fallback exists, that branch is equivalent to the fallback — \n, \r, \t fall through and are emitted verbatim either way (\v and \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_cells pattern already in the file. assert "@" in output would 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.

@romanlutz
romanlutz added this pull request to the merge queue Aug 2, 2026
Merged via the queue into microsoft:main with commit 446e47b Aug 2, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: BrailleConverter silently drops characters outside its lookup table

3 participants