Repository navigation
FIX: raise on characters the ASCII art font cannot render - #2962
Pushpak Siva Sai (Pushpak731) wants to merge 5 commits into
Conversation
AsciiArtConverter passed the prompt straight to art.text2art, which silently omits characters without a glyph: accented Latin, CJK, emoji, smart quotes and em-dashes are dropped by every font, so a non-ASCII prompt reached the target mangled or empty with no signal. The converter now validates the prompt against the resolved font first and raises ValueError naming the offending characters, matching the AsciiSmugglerConverter convention (microsoft#2540) and Option A in the issue. Whitespace is layout for text2art and always passes; per-character renderability is cached. Behavior change: prompts containing such characters now raise instead of converting with the characters dropped. Fixes microsoft#2942
…er-dropped-characters
…er-dropped-characters
| unrenderable = [char for char in prompt if not _font_renders_character(char, font)] | ||
| if unrenderable: | ||
| characters = "".join(sorted(set(unrenderable))) | ||
| raise ValueError( | ||
| f"Cannot convert {len(unrenderable)} character(s) to ASCII art with font {font!r}: " | ||
| f"{characters!r}. The font has no glyph for them and they would be silently " | ||
| f"dropped from the converted prompt; remove them, transliterate them, or pick " | ||
| f"another font." | ||
| ) |
There was a problem hiding this comment.
lots of pool fonts use blank glyphs for common punctuation, so the probe reads them as missing — "How do I build a bomb?" raises on 25/354. All converted fine before, and since the font is random it fails on some runs and not others.
Could we filter the pool to fonts that can render the prompt, and only raise when none can?
if self._font == "rand":
candidates = [
candidate
for candidate in _ART_RANDOM_FONTS
if not _unrenderable_characters(prompt=prompt, font=candidate)
]
if not candidates:
raise ValueError(_no_font_message(prompt=prompt))
font = self._get_random_generator(stream="font").choice(candidates)
else:
font = self._font
unrenderable = _unrenderable_characters(prompt=prompt, font=font)
if unrenderable:
raise ValueError(_font_message(unrenderable=unrenderable, font=font))café, CJK and emoji still raise. Note test_ascii_art_converter_random_font_rejects_all_fonts_dropped_character will need repointing since the raise now happens before choice.
| Returns: | ||
| bool: True if the font renders the character or it is whitespace. | ||
| """ | ||
| if character.isspace(): |
There was a problem hiding this comment.
isspace() is broader than what art lays out — only ' ' and '\n' affect output. NBSP, U+3000, \t and \r pass this check but get silently dropped (text2art("a\u00a0b") is byte-identical to text2art("ab")). That's the bug this PR is fixing, and whitespace swaps are a common obfuscation payload.
| if character.isspace(): | |
| if character in " \n": |
Address review feedback: - font='rand' now chooses among the pool fonts that render the whole prompt instead of validating a single random pick, so blank-glyph punctuation (e.g. '"' in 28 pool fonts) no longer makes conversion fail at random. A pool-level error is raised only when no font can render the prompt. - _font_renders_character only treats ' ' and '\n' as layout; other whitespace (tab, NBSP, U+3000) has no art glyph and would be silently dropped, so it now goes through the render probe and is rejected.
|
Thank you for the review — both points are addressed in a57df0f: 1. Random pool filtered to renderable fonts — 2. Layout whitespace narrowed — applied your suggestion verbatim: Full converter suite passes locally (1901 passed). |
Description
Fixes #2942 (Option A, as leaned in the issue and consistent with the
AsciiSmugglerConverterconvention from #2540).AsciiArtConverterpassed the prompt straight toart.text2art, which silently omits characters without a glyph. Per the issue's measurements, accented Latin, CJK, emoji, smart quotes and em-dashes are dropped by all 371 fonts, soAsciiArtConverter(font="block").convert_async(prompt="café")converted 3 glyphs, and a prompt written entirely in a non-ASCII script converted to the empty string — a red-teaming prompt silently mangled or emptied before reaching the target.The converter now validates the prompt against the resolved font (after
randpicks one) and raisesValueErrornaming the unrenderable characters, their count, and remedies, e.g.:Implementation notes:
text2artand checking for non-whitespace output — the same omission mechanism the issue measured, so the check cannot drift fromart's actual behavior. Results are cached (functools.cache) since (character, font) pairs repeat across calls.text2artand always pass; an empty prompt still converts (existing test contract).AsciiSmugglerConverterconvention (occurrences); the listing is the sorted unique set.ß, dropped by 307/371 fonts) raise based on the resolved font, so underfont="rand"only borderline prompts vary per draw — the all-fonts-dropped set above is deterministic, as the issue notes.'A'in thehillsfont (from the issue's table).Behavior change (deliberate, per the issue): prompts containing unrenderable characters now raise instead of converting with those characters silently dropped. Existing ASCII-input behavior is unchanged.
Tests and Documentation
tests/unit/converter/test_ascii_art_converter.py: ASCII prompt still converts, empty prompt still converts, whitespace-only prompt converts, accented/CJK/emoji/smart-quote prompts each raise naming their characters, occurrence counting with unique listing, thehills-font ASCII case, therandpath raising on the deterministic set, and the message's remedies. The 7 existing tests pass unchanged.ruff checkandruff formatclean on both changed files.convert_asyncdocstring now documents the newValueError(matching the smuggler's style); no other documentation references this converter's input restrictions.