Handle XML-illegal control characters in source/target/note text consistently
- Dominant language
- Ruby
- Stars
- 3
- Forks
- 2
- Avg merge
- 2h 46m
- Merged PRs (30d)
- 3
Description
XML 1.0 forbids the C0 control characters (`0x00`–`0x1F` except tab, LF, and CR) anywhere in a document, including text content. `Xliff` neither rejects nor consistently sanitizes them in `source`/`target`/`note` text, and the two serialization paths disagree on what happens — so the same string produces different (both lossy) output depending on which one you call.
Confirmed with a form feed (`0x0c`) in `source`:
- **`Entry#to_s`** replaces it with the Unicode replacement char: `Line1�Line2` — lossy, but valid XML.
- **`Bundle#to_s`** emits the **raw `0x0c` byte** into the output — which is not well-formed XML 1.0 (Nokogiri itself rejects it: `FATAL: PCDATA invalid Char value 12`). On re-parse the library recovers by **silently dropping** the character, so `Bundle.from_string(bundle.to_s)` loses data:
```ruby
ff = "Line1\x0cLine2"
Xliff::Entry.new(id: "x", source: ff, target: "T").to_s
#=> "Line1�Line2..." # U+FFFD
b = Xliff::Bundle.new
f = Xliff::File.new(original: "x", source_language: "en")
f.add_entry(Xliff::Entry.new(id: "ctrl", source: ff))
b.add_file(f)
out = b.to_s
out.bytes.include?(0x0c) #=> true (raw, invalid byte emitted)
Xliff::Bundle.from_string(out).files[0].entries[0].source #=> "Line1Line2" (char vanished)
```
So `Bundle#to_s` can emit a document a conformant parser rejects, and a tolerant re-parse silently corrupts the text — different from `Entry#to_s`'s `U+FFFD`.
## Proposed fix
Pick one policy and apply it in a single shared place (the text path that both `Entry#to_s` and `Bundle#to_s` flow through), so the two paths can't diverge:
1. **Reject** — raise on an XML-illegal control character at construction/parse. Consistent with the rest of #15 ("surface malformed input instead of fabricating it"): a string that can't be represented in XLIFF 1.2 is malformed input, not something to silently mangle.
2. **Sanitize** — strip or replace illegal chars consistently (e.g. always `U+FFFD`, matching the current `Entry#to_s` behavior), and document it as lossy.
Rejecting is the better fit for the library's current stance; sanitizing is the more permissive option if callers are expected to pass through arbitrary upstream text. Either way the two serialization paths must agree.
## Scope
Split out of the #15 review sweep — text encoding is **pre-existing** and untouched by that PR. Xcode's own exports don't contain control characters, so this isn't required for that use case, but it's silent data loss (and invalid output) for general input, and the path divergence is a latent footgun. Same "decide a policy, apply it once" shape as the conformance fixes in #15.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by tracing the text serialization paths used by Entry#to_s and Bundle#to_s, then reproduce the form-feed example from the issue. Decide with the maintainer whether illegal XML characters should be rejected or sanitized, and apply that policy through their shared path. Done means both serializers agree, output is well-formed XML, and the behavior is covered for source, target, and note text.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- ruby, xml
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100