Skip to content

fix: stop double-escaping what a provider sends back - #24

Merged
Halvanhelv merged 3 commits into
mainfrom
fix/provider-entity-decoding
Sep 11, 2026
Merged

fix: stop double-escaping what a provider sends back#24
Halvanhelv merged 3 commits into
mainfrom
fix/provider-entity-decoding

Conversation

@Halvanhelv

Copy link
Copy Markdown
Owner

Found by translating a real HTML article through the live Google and DeepL
APIs — not by a test, and not by three rounds of review before it.

What was broken

Every translation into English through Google or DeepL came back corrupted
wherever it contained an apostrophe, a quote or an ampersand:

in : <p>Он не увёл лодку.</p>
out: <p>He didn&amp;#39;t take the boat away.</p>

which renders on the page as didn&#39;t. English is full of apostrophes,
so in practice every Google or DeepL translation into English was damaged
somewhere.

Both vendors return entity-escaped HTML — confirmed against the raw APIs,
not inferred. The pipeline decoded entities on the way in, so a provider is
sent characters rather than entities, and re-encoded & and < at render
— but never decoded what the provider sent back. The vendor's & was
treated as a literal ampersand and escaped a second time.

The fix decodes a provider's returned text with the same decoder the input
path uses, in Translation::Response.build — the one place every provider's
output passes through, including the two that override translate entirely,
and including a provider somebody writes themselves.

The hole that opened, and closed

Decoding the vendor's &lt; produced a bare <, and a bare < in front of
a letter is a tag. A provider returning &lt;b attack put a real <b into
the rendered document:

<p>Value <b attack here.</p>      # tags found: ["<p>", "<b attack here.</p>"]

The second commit closes it: a translated < that is not shaped like a tag
is escaped on render, so a provider's markup survives and a provider's text
cannot become markup. The test that would have caught it is now there — the
tag count and tag sequence of a real HTML article must be identical before
and after a round trip through a vendor that escapes.

What changes for a user

A literal < in the source now comes back as &lt; rather than bare.
if a < b then stop. becomes if a &lt; b then stop., which is the correct
HTML encoding of that character and renders identically. Byte-for-byte
comparison against a previous version will show it. > is left alone; it is
unambiguous.

Warm caches keep the old text. A cache key is derived from the source
sentence, not from the stored value, so entries written before this fix keep
being served until they expire. Bump cache_namespace, or let cache_ttl
lapse.

Verified live

Both vendors, ru→en, through the gem's public API:

<p>He didn't take the boat.</p>
<p>5 &amp; 7 — these are important.</p>
<p>Compare: 5 &lt; 7 &amp;&amp; 7 > 5.</p>

A 68-tag article — lists, a table, a blockquote, a figure, an HTML comment,
an attribute carrying &amp;, a notranslate span — round-trips with the
tag sequence identical, the span untouched, the link's attributes intact and
its text translated.

Also documented, because the same article exposed it: nothing tells the gem
that <pre> and <code> are code, so their contents are translated like
prose. class="notranslate" is the remedy.

684 runs, rubocop clean.

Google and DeepL HTML-escape every reply, so an apostrophe, quote or
ampersand in the translation came back as &#39;, &quot; or &amp;. Our
own render step re-escaped the leading & of each, corrupting the page
with visible junk like &amp;#39;.

Response.build is where every provider's texts already converge to be
shape-checked, so it decodes them there too -- symmetric with the
existing input-side decode, and automatic for a third-party provider
without a six-way per-provider hook.

Verified live against both vendors: apostrophes, quotes and ampersands
now round-trip correctly, and a notranslate span with an entity is
unaffected. Cache entries written before this fix keep the doubled
text until they expire; bump cache_namespace or let cache_ttl lapse to
clear them.
Decoding a provider's entities (previous commit) exposed a second
bug: Passage renders every fragment then runs one restore pass over
the whole string to undo the escaping it applied to the source's own
bare angles. A translated `&lt;` looked exactly like that source-level
escaping to that pass, so it got "restored" to a bare `<` -- and a
bare `<` in front of a letter reads as an opening tag. A provider's
own `&lt;b attack` came back as a real `<b attack>` element.

Response.build now escapes a provider's raw text the same way the
input path escapes @Body before decoding it, so an entity the
provider genuinely sent survives as the entity it is rather than
the character it decodes to. Segment#render pairs this with a new
Markup.encode_translation: it re-escapes a translation the same way
encode_entities does, except it leaves a `<` shaped like a tag alone,
trusting a provider's reproduced markup (a notranslate span, say) the
same way a source tag already is.

Verified live against both vendors: the apostrophe/quote fix from the
previous commit still holds, and "5 &lt; 7 &amp;&amp; 7 &gt; 5" now
comes back with its entities intact rather than as bare `<`/`&`.
Covers what fix/provider-entity-decoding changed for users: the
double-escaping corruption is gone, a translated bare < now renders as
&lt; (a behaviour change), and a warm cache keeps serving the old
corrupted text until it expires or cache_namespace changes. Also notes
that <pre>/<code> blocks are ordinary prose to this gem and are
translated unless wrapped in class="notranslate", measured against the
live Google API.
@Halvanhelv
Halvanhelv merged commit 1756d21 into main Sep 11, 2026
5 checks passed
@Halvanhelv
Halvanhelv deleted the fix/provider-entity-decoding branch September 11, 2026 01:53
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.

1 participant