From 28c056c6401b77d5b2d016df800c720f4ad82c33 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 03:41:19 +0400 Subject: [PATCH 1/3] fix(translation): Decode a provider's returned entities once Google and DeepL HTML-escape every reply, so an apostrophe, quote or ampersand in the translation came back as ', " or &. Our own render step re-escaped the leading & of each, corrupting the page with visible junk like &#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. --- lib/translation_diff/translation/response.rb | 4 +- test/translation_diff/markup_test.rb | 33 +++++++++++++++++ test/translation_diff/providers/null_test.rb | 8 ++++ .../translation/response_test.rb | 37 +++++++++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/translation_diff/translation/response.rb b/lib/translation_diff/translation/response.rb index 3829c2f..5b05aff 100644 --- a/lib/translation_diff/translation/response.rb +++ b/lib/translation_diff/translation/response.rb @@ -5,7 +5,9 @@ def self.build(request:, texts:, detected_source: nil, usage: nil) ensure_count!(request, texts) ensure_strings!(texts) - new(texts: texts, detected_source: detected_source, usage: usage) + # Every provider's text lands here, so the same decoder the input path used runs once, here, on the way back. + new(texts: texts.map { |text| TranslationDiff::Markup.decode_entities(text) }, + detected_source: detected_source, usage: usage) end def self.ensure_count!(request, texts) diff --git a/test/translation_diff/markup_test.rb b/test/translation_diff/markup_test.rb index 400f408..381ef5a 100644 --- a/test/translation_diff/markup_test.rb +++ b/test/translation_diff/markup_test.rb @@ -20,6 +20,15 @@ def echoed(source) subject.render end + # What Google and DeepL do: html-escape whatever text they hand back, then what Response.build now undoes. + def vendor_escaped(source) + subject = passage(source) + subject.segments.reject(&:empty?).each do |s| + s.translation = TranslationDiff::Markup.decode_entities(CGI.escapeHTML(s.core)) + end + subject.render + end + def assert_round_trips(source) assert_equal source, passage(source).render, "render must return the source byte for byte" end @@ -191,4 +200,28 @@ def test_an_entity_inside_markup_is_left_for_the_browser assert_round_trips(%(Link text. After.)) assert_round_trips("Before.After.") end + + # -- a vendor's own escaping ----------------------------------------------- + + # Reproduces the shipped bug: Google and DeepL html-escape every reply, apostrophes and quotes included. + def test_a_vendor_that_escapes_apostrophes_and_quotes_round_trips_clean + assert_equal "He didn't take the boat.", vendor_escaped("He didn't take the boat.") + assert_equal %(She said, "We're not ready."), vendor_escaped(%(She said, "We're not ready.")) + end + + # An ampersand a vendor escaped is undone once, then re-escaped once at render -- never doubled either way. + def test_a_vendor_escaped_ampersand_is_not_doubled + assert_equal "5 & 7 are important.", vendor_escaped("5 & 7 are important.") + end + + # The notranslate span's own text is sent and returned like any other sentence, entity and all. + def test_a_vendor_escaped_notranslate_span_keeps_its_ampersand_readable + assert_equal %(R&D Fine.), + vendor_escaped(%(R&D Fine.)) + end + + # One decode pass, never two: a reply already doubly-escaped loses only the level the wire itself added. + def test_decoding_a_double_encoded_reply_removes_only_one_level + assert_equal "&", TranslationDiff::Markup.decode_entities("&amp;") + end end diff --git a/test/translation_diff/providers/null_test.rb b/test/translation_diff/providers/null_test.rb index 567c563..4192d60 100644 --- a/test/translation_diff/providers/null_test.rb +++ b/test/translation_diff/providers/null_test.rb @@ -14,6 +14,14 @@ def test_translate_returns_the_input_unchanged assert_equal %w[one two], provider.translate(request).texts end + # Response.build's new decoding step must be a no-op for the one provider that never escapes anything. + def test_translate_does_not_touch_text_that_only_looks_like_it_needs_decoding + texts = ["AT&T merged.", "5 < 7 is true.", "He didn't go."] + request = TranslationDiff::Translation::Request.new(texts: texts, from: :en, to: :ru) + + assert_equal texts, provider.translate(request).texts + end + def test_cache_key_is_null assert_equal "null", provider.cache_key end diff --git a/test/translation_diff/translation/response_test.rb b/test/translation_diff/translation/response_test.rb index 96bc511..fd2a125 100644 --- a/test/translation_diff/translation/response_test.rb +++ b/test/translation_diff/translation/response_test.rb @@ -15,6 +15,43 @@ def test_build_returns_the_texts_it_was_given assert_equal %w[один два], response.texts end + # -- decoding what a provider sent back ----------------------------------- + + # Google and DeepL both HTML-escape their output; undoing that here is what made the round trip symmetric. + def test_build_decodes_the_apostrophe_and_quote_a_provider_escaped + response = TranslationDiff::Translation::Response.build( + request: request, texts: ["He didn't say "hi".", "5 & 7."] + ) + + assert_equal ["He didn't say \"hi\".", "5 & 7."], response.texts + end + + # Numeric, hex and named entities all decode; an entity outside the known set is left exactly as it arrived. + def test_build_decodes_numeric_hex_and_named_entities_and_leaves_an_unknown_one_alone + response = TranslationDiff::Translation::Response.build( + request: request(%w[one]), texts: ["' ' & "   — &nosuch;"] + ) + + assert_equal ["' ' & \" \u00A0 \u2014 &nosuch;"], response.texts + end + + # A provider that never escapes its output -- the :null provider, or any other -- must not have a character + # that merely looks like the start of an entity eaten; only a well-formed entity is ever touched. + def test_build_leaves_a_non_escaping_providers_output_untouched + response = TranslationDiff::Translation::Response.build( + request: request, texts: ["AT&T merged.", "5 < 7 is true."] + ) + + assert_equal ["AT&T merged.", "5 < 7 is true."], response.texts + end + + # One decode pass, never two -- a doubly-escaped reply loses only the level the wire itself added. + def test_build_decodes_a_double_escaped_reply_only_once + response = TranslationDiff::Translation::Response.build(request: request(%w[one]), texts: ["&amp;"]) + + assert_equal ["&"], response.texts + end + # A short response would shift nils into the results, surfacing much later as a distant NoMethodError. def test_build_raises_when_the_provider_returned_the_wrong_number_of_texts error = assert_raises(TranslationDiff::ResponseError) do From ac52c4862f72f6d4d827eb39da842d4d6e998656 Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 04:04:58 +0400 Subject: [PATCH 2/3] fix(translation): Keep a decoded < from ever reading as a tag 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 `<` 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 `<b attack` came back as a real `` 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 < 7 && 7 > 5" now comes back with its entities intact rather than as bare `<`/`&`. --- lib/translation_diff/markup.rb | 9 ++ lib/translation_diff/segment.rb | 10 +- lib/translation_diff/translation/response.rb | 10 +- test/translation_diff/markup_test.rb | 2 +- test/translation_diff/pipeline_corpus_test.rb | 9 +- .../provider_entity_decoding_test.rb | 93 +++++++++++++++++++ 6 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 test/translation_diff/provider_entity_decoding_test.rb diff --git a/lib/translation_diff/markup.rb b/lib/translation_diff/markup.rb index e359922..5f0adf1 100644 --- a/lib/translation_diff/markup.rb +++ b/lib/translation_diff/markup.rb @@ -67,6 +67,15 @@ def self.resolve(name) # What a document renders is markup, so text that changed is made safe again -- and only where it is unsafe. def self.encode_entities(text) = text.gsub(ENCODABLE, ENCODED) + # An entity the round trip already produced must not be escaped a second time, and a `<` shaped like a tag is + # trusted the same way a source tag already is -- everything else a provider sent back is untrusted new text. + TRANSLATED_ENCODABLE = /&(?:amp|lt|gt);|&|<(?!#{TAG_OPENER})/ + + # What a translation renders as: unlike #encode_entities, this leaves a provider's own reproduced tags alone. + def self.encode_translation(text) + text.gsub(TRANSLATED_ENCODABLE) { |match| match.length == 1 ? ENCODED[match] : match } + end + # Ox hands back the decoded text of the one element it was given; a name it does not know arrives as the text it was. class Resolver < Ox::Sax attr_reader :text diff --git a/lib/translation_diff/segment.rb b/lib/translation_diff/segment.rb index 9b04f0a..1d34c65 100644 --- a/lib/translation_diff/segment.rb +++ b/lib/translation_diff/segment.rb @@ -19,6 +19,14 @@ def empty? = core.match?(BLANK) # Untranslated hands back the bytes it was cut from; a translation is text, so it is encoded as markup on the way out. def render - "#{@leading}#{translated? ? TranslationDiff::Markup.encode_entities(translation) : @body}#{@trailing}" + "#{@leading}#{translated? ? escaped_translation : @body}#{@trailing}" + end + + private + + # @body already carries this same escape from Passage; without it, Passage's one shared restore pass would + # read a translated `<` as a source document's own bare `<` and hand back markup nobody asked for. + def escaped_translation + TranslationDiff::Markup.escape_bare_angles(TranslationDiff::Markup.encode_translation(translation)) end end diff --git a/lib/translation_diff/translation/response.rb b/lib/translation_diff/translation/response.rb index 5b05aff..8646b0d 100644 --- a/lib/translation_diff/translation/response.rb +++ b/lib/translation_diff/translation/response.rb @@ -5,9 +5,13 @@ def self.build(request:, texts:, detected_source: nil, usage: nil) ensure_count!(request, texts) ensure_strings!(texts) - # Every provider's text lands here, so the same decoder the input path used runs once, here, on the way back. - new(texts: texts.map { |text| TranslationDiff::Markup.decode_entities(text) }, - detected_source: detected_source, usage: usage) + # Every provider's text lands here, so it takes the same path @core did: escaped, then decoded once, so an + # entity a provider genuinely sent survives as the entity it is rather than the bare character it decodes to. + new(texts: texts.map { |text| decoded(text) }, detected_source: detected_source, usage: usage) + end + + def self.decoded(text) + TranslationDiff::Markup.decode_entities(TranslationDiff::Markup.escape_bare_angles(text)) end def self.ensure_count!(request, texts) diff --git a/test/translation_diff/markup_test.rb b/test/translation_diff/markup_test.rb index 381ef5a..c5db917 100644 --- a/test/translation_diff/markup_test.rb +++ b/test/translation_diff/markup_test.rb @@ -75,7 +75,7 @@ def test_a_bare_less_than_survives_rendering_untranslated # the same string also contains a bare <. def test_a_real_tag_beside_a_bare_less_than_is_still_markup assert_equal ["if a < b then", "stop."], cores("if a < b then stop.") - assert_equal "IF A < B THEN STOP.", translated("if a < b then stop.") + assert_equal "IF A < B THEN STOP.", translated("if a < b then stop.") end def test_a_less_than_immediately_before_a_letter_is_a_tag diff --git a/test/translation_diff/pipeline_corpus_test.rb b/test/translation_diff/pipeline_corpus_test.rb index 01b083c..f4557a1 100644 --- a/test/translation_diff/pipeline_corpus_test.rb +++ b/test/translation_diff/pipeline_corpus_test.rb @@ -60,15 +60,16 @@ def self.method_name_for(name) = :"test_#{name.gsub(/[^a-zA-Z0-9]+/, '_')}" document: "Hard\u00A0space here. Fine.", echoed: "Hard\u00A0space here. Fine." }, + # A translated `<` not shaped like a tag is escaped now, so it can never read back as one after this fix. "bare less-than" => { texts: ["if a < b then stop.", "Fine."], - document: "if a < b then stop. Fine.", - echoed: "if a < b then stop. Fine." + document: "if a < b then stop. Fine.", + echoed: "if a < b then stop. Fine." }, "bare less-than and greater" => { texts: ["5 < 6 and 7 > 6.", "True."], - document: "5 < 6 and 7 > 6. True.", - echoed: "5 < 6 and 7 > 6. True." + document: "5 < 6 and 7 > 6. True.", + echoed: "5 < 6 and 7 > 6. True." }, # The recorded limit: ` { diff --git a/test/translation_diff/provider_entity_decoding_test.rb b/test/translation_diff/provider_entity_decoding_test.rb new file mode 100644 index 0000000..8084f38 --- /dev/null +++ b/test/translation_diff/provider_entity_decoding_test.rb @@ -0,0 +1,93 @@ +require "test_helper" + +# Round 2 of the entity-decoding fix: a provider's own `<` must never reach the page as a bare, tag-forming `<`. +class ProviderEntityDecodingTest < ConfiguredTest + # What a real vendor does: escape a literal `&` and a `<` that opens no tag; a genuine tag is left alone. + def self.vendor_escape(text) + text.gsub(/&|<(?!#{TranslationDiff::Markup::TAG_OPENER})/) { |match| match == "&" ? "&" : "<" } + end + + # Mimics Google/DeepL: html-aware on the way in too, so an `<` sent as safe HTML is read as the character + # it means, not retranslated as four literal letters, before the reply is escaped the way a real vendor's is. + class EscapingProvider < TranslationDiff::Provider + def self.capabilities + TranslationDiff::Capabilities.new( + max_request_size: 1_000, max_batch_size: 10, max_text_size: nil, + html: :format, notranslate: true, detects_language: false, reports_billing: false + ) + end + + def translate(request) + texts = request.texts.map { |text| ProviderEntityDecodingTest.vendor_escape(TranslationDiff::Markup.decode_entities(text)) } + TranslationDiff::Translation::Response.build(request: request, texts: texts) + end + + def cache_key = "escaping" + end + + # A provider that ignores what it is sent and returns a fixed string -- for reproducing the injection directly. + class FixedProvider < TranslationDiff::Provider + def self.capabilities = EscapingProvider.capabilities + + def initialize(config, text:) + super(config) + @text = text + end + + def translate(request) + TranslationDiff::Translation::Response.build(request: request, texts: request.texts.map { @text }) + end + + def cache_key = "fixed" + end + + def tags_in(html) = html.scan(%r{]*>}) + + def translate(source, provider) + TranslationDiff.translate(source, from: "ru", to: "en", provider: provider) + end + + # The shipped hole: a provider's own `<b attack` must never become a real, page-breaking `` tag. + def test_a_dangerous_tag_shaped_entity_never_forms_a_real_tag + provider = FixedProvider.new(TranslationDiff::Configuration.new, text: "Value <b attack here.") + output = translate("

Value X here.

", provider) + + assert_includes output, "<b attack" + assert_equal ["

", "

"], tags_in(output) + end + + # DeepL/Google's own reproduction case: `<` and `&` come back as entities -- not bare, not doubled. + # `>` was never escaped by this gem, translated or not, so it stays literal; only `<` and `&` are at stake. + def test_a_vendor_that_escapes_preserves_comparison_entities + provider = EscapingProvider.new(TranslationDiff::Configuration.new) + output = translate("

Сравните: 5 < 7 && 7 > 5.

", provider) + + assert_equal "

Сравните: 5 < 7 && 7 > 5.

", output + end + + # The apostrophe/quote corruption this branch already fixed must stay fixed alongside the new tag protection. + def test_apostrophe_and_quote_corruption_stays_fixed + provider = EscapingProvider.new(TranslationDiff::Configuration.new) + + assert_equal "

He didn't take the boat away.

", translate("

He didn't take the boat away.

", provider) + assert_equal "

5 & 7 are important.

", translate("

5 & 7 are important.

", provider) + end + + # The property that would have caught this: a real article's tags, count and sequence, survive the round trip. + ARTICLE = <<~HTML.chomp +
+

Guide to Shell Quoting

+

Compare: 5 < 7 and check the & operator carefully.

+

Run jq '.meters' to extract the field.

+

Read more about the topic.

+

The vendor TrustedCo provided this data.

+
+ HTML + + def test_a_real_articles_tag_count_and_sequence_survive_a_round_trip + provider = EscapingProvider.new(TranslationDiff::Configuration.new) + output = translate(ARTICLE, provider) + + assert_equal tags_in(ARTICLE), tags_in(output) + end +end From df15136ed7e36e5d7620347f01f5c1e8e1d9229b Mon Sep 17 00:00:00 2001 From: IG Date: Fri, 11 Sep 2026 04:14:47 +0400 Subject: [PATCH 3/3] docs: document the entity round-trip fix and its two consequences Covers what fix/provider-entity-decoding changed for users: the double-escaping corruption is gone, a translated bare < now renders as < (a behaviour change), and a warm cache keeps serving the old corrupted text until it expires or cache_namespace changes. Also notes that 
/ blocks are ordinary prose to this gem and are
translated unless wrapped in class="notranslate", measured against the
live Google API.
---
 CHANGELOG.md         | 35 +++++++++++++++++++++++++++++++++
 docs/caching.md      |  9 +++++++++
 docs/how-it-works.md | 46 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 481b631..f17ceac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -141,6 +141,41 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   each other -- see
   [The three write paths fail differently](docs/caching.md#the-three-write-paths-fail-differently).
 
+### Fixed
+
+- **Google and DeepL translations in HTML mode no longer come back
+  double-escaped.** Both vendors return entity-escaped text -- an
+  apostrophe as `'`, a quote as `"`, an ampersand as `&` -- and
+  the pipeline decoded entities on the way in but never on the way out, so
+  the renderer escaped the vendor's own `&` a second time and a reader saw
+  `didn't` on the page. English is full of apostrophes, so in practice
+  every Google or DeepL translation into English was affected somewhere.
+  `TranslationDiff::Translation::Response.build` now decodes a provider's
+  reply the same way it already decoded the source, symmetrically, for
+  every provider -- named entities, and both the decimal (`'`) and hex
+  (`'`) numeric forms, are decoded; an entity neither decoder
+  recognizes is left exactly as it arrived. See [How it
+  works](docs/how-it-works.md).
+- **Behaviour change: a literal `<` in a source sentence now renders as
+  `<`.** Decoding the fix above exposed a second bug: a provider's own
+  `<` now decoded to a bare `<`, and a bare `<` in front of a letter
+  reads as an opening tag -- a provider could inject markup into the
+  rendered document. A translated `<` that is not shaped like a tag is now
+  escaped on render instead. `if a < b then stop.` used to come back with
+  the bare `<` exactly as written; it now comes back
+  `if a < b then stop.`, the correct HTML encoding of that character and
+  identical once a browser renders it -- but visible to anything comparing
+  output byte-for-byte against an earlier release. `>` is untouched: a
+  stray `>` never opens anything a parser would honour. See [How it
+  works](docs/how-it-works.md#html).
+- **A warm cache keeps serving the corrupted text after you upgrade.** A
+  cache entry's key is derived from the source sentence, not from the value
+  stored under it, so an entry written before this fix is served exactly as
+  it was written until it expires -- upgrading alone does not clear it.
+  Give the configuration a new `cache_namespace`, or let `cache_ttl` lapse,
+  to force every sentence to be retranslated under the fix. See
+  [Caching](docs/caching.md).
+
 ### Security
 
 - `Configuration#inspect` and `Provider#inspect` print `[FILTERED]` in place
diff --git a/docs/caching.md b/docs/caching.md
index af70ddc..5834f64 100644
--- a/docs/caching.md
+++ b/docs/caching.md
@@ -17,6 +17,15 @@ its own Redis database). The key format is left alone here on purpose:
 changing its shape invalidates every entry already cached, everywhere, at
 once.
 
+**A cache entry written before a bug fix keeps serving what the bug
+produced.** The key above is built from the source sentence, never from the
+value stored under it, so fixing what a provider's reply decodes to does not
+invalidate what is already cached -- an entry written under the HTML-entity
+double-escaping fixed in the Unreleased CHANGELOG entry is served exactly as
+it was written until it expires. Give the configuration a new
+`cache_namespace`, or let `cache_ttl` lapse, to force every sentence to be
+retranslated under the fix.
+
 Both read and write the same cache, keyed per provider, so switching one
 never serves you the other's translations.
 
diff --git a/docs/how-it-works.md b/docs/how-it-works.md
index 6406eaf..3bd71e5 100644
--- a/docs/how-it-works.md
+++ b/docs/how-it-works.md
@@ -59,9 +59,29 @@ Everything below is a collaborator one of the two drives.
    shape.
 
 `TranslationDiff::Markup` is the small module underneath steps 2, 3 and 6: it
-decodes entity references on the way to a provider, encodes `&` and `<` again
-on the way out, and escapes a `<` that opens no tag so `ox` cannot read the
-rest of the sentence as markup.
+decodes entity references on the way to a provider and, in
+`TranslationDiff::Translation::Response.build`, on the way back too, for
+every provider -- Google and DeepL both return HTML-escaped text, and
+without the second decode a vendor's own `&` was escaped a second time, so
+`didn't` came back as `didn't`. Named entities, and both the decimal
+(`'`) and hex (`'`) numeric forms, are decoded; an entity neither
+decoder recognizes, or one that would decode to invalid UTF-8, is left
+exactly as it arrived.
+
+Decoding a reply raw would make `<` a bare `<`, and `ox` reads a bare `<`
+in front of a letter as an opening tag -- a provider's own `<b attack`
+would become a real `` element. So a reply is escaped the same way
+a source document's own bare angles already are, before it is decoded, and
+`Segment#render` re-encodes a translated sentence with
+`Markup.encode_translation`: `&` is always escaped, and so is a `<` that is
+not shaped like a tag -- a source document's own bare `<` is untouched by
+this. **This is a behaviour change:** `if a < b then stop.` used to come
+back with the bare `<` exactly as written; it now comes back
+`if a < b then stop.`, the correct HTML encoding of that character,
+rendering identically in a browser but visible to anything comparing output
+byte-for-byte against an earlier release. `>` is left alone -- a stray `>`
+never opens anything a parser would honour, so there is nothing to protect
+it from.
 
 *NOTE:* if `:from` is not specified or equal to nil, then the provider's `#detect` will be called once with a sample of text up to 100 characters long to determine the language, and `#translate` will be called separately with the entire text.
         Try to specify `:from` explicitly to save the extra call -- it also improves segmentation, since the segmenter only sees a language when `:from` is given (see [The segmenter contract](contracts.md#the-segmenter-contract)).
@@ -93,3 +113,23 @@ You can pass HTML as like as plain text:
 ```ruby
 TranslationDiff.translate("Black", from: "en", to: "es")
 ```
+
+Nothing marks a `
` or `` block as code. The scanner's `OPAQUE`
+list (see [The steps](#the-steps) above) excludes only `