Skip to content

Commit 6d12559

Browse files
committed
fix: strip invalid UTF-8 characters of any length, not just sequences
The non-ocr path in drawChar() only tested sequences longer than one character, so a single invalid character was emitted verbatim and could make the ALTO output non-well-formed. Ported from the stale branch bugfix/utf8-invalid-characters_placeholder; that branch's -ocr -> -placeholder rename is obsolete here, since -ocr now means "emit an OCR sidecar" rather than "substitute placeholders". isUTF8() encoded the sequence through the UTF-8 UnicodeMap and then re-parsed the resulting bytes. That round trip was redundant: mapUTF8() encodes each codepoint independently and well-formed by construction, so the only input it can render invalid is an unpaired surrogate, and codepoints above U+10FFFF encode to zero bytes and so were, and still are, reported as valid. Replaced with the equivalent integer test, verified against the old scan over every codepoint U+0000..U+10FFFF and over multi-character sequences. This also drops a GString that leaked on all nine of the old function's return paths, which matters now that the function runs once per character rather than only for uLen > 1. Verified on a 14-PDF corpus: the isUTF8 rewrite alone is byte-identical to the previous output, and neither strip path fires on any of those documents, so the guard fix is latent until a document carries a lone surrogate.
1 parent 342c069 commit 6d12559

1 file changed

Lines changed: 23 additions & 84 deletions

File tree

src/XmlAltoOutputDev.cc

Lines changed: 23 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -9716,8 +9716,9 @@ void XmlAltoOutputDev::drawChar(GfxState *state, double x, double y, double dx,
97169716
if (parameters->getOcr() == gTrue) {
97179717
if ((uLen == 0 ||
97189718
((u[0] == (Unicode) 0 || u[0] < (Unicode) 32) && uLen == 1) ||
9719-
(uLen > 1 && (globalParams->getTextEncodingName()->cmp(ENCODING_UTF8)==0)&& !isUTF8(u, uLen)))) {
9720-
//when len is gt 1 check if sequence is valid, if not replace by placeholder
9719+
((globalParams->getTextEncodingName()->cmp(ENCODING_UTF8)==0)&& !isUTF8(u, uLen)))) {
9720+
// Check the sequence is valid whatever its length: a single invalid
9721+
// character needs a placeholder just as much as a multi-char one.
97219722
//&& globalParams->getApplyOCR())
97229723
// as a first iteration for dictionnaries, placing a placeholder, which means creating a map based on the font-code mapping to unicode from : https://unicode.org/charts/PDF/U2B00.pdf
97239724
GString *fontName;
@@ -9759,7 +9760,10 @@ void XmlAltoOutputDev::drawChar(GfxState *state, double x, double y, double dx,
97599760
GString *rawFontName = gfxFont ? gfxFont->getName() : NULL;
97609761
ocrFontName = rawFontName ? rawFontName->getCString() : "";
97619762
}
9762-
} else if(uLen > 1 && (globalParams->getTextEncodingName()->cmp(ENCODING_UTF8)==0)&& !isUTF8(u, uLen))
9763+
// Without -ocr, strip every invalid UTF-8 character rather than emitting it:
9764+
// a single one is enough to make the ALTO output non-well-formed XML. The
9765+
// former uLen > 1 guard let exactly those single characters through.
9766+
} else if((globalParams->getTextEncodingName()->cmp(ENCODING_UTF8)==0) && !isUTF8(u, uLen))
97639767
return;
97649768

97659769
text->addChar(state, x, y, dx, dy, c, nBytes, u, uLen, splashFont, isNonUnicodeGlyph);
@@ -10755,86 +10759,21 @@ void XmlAltoOutputDev::tilingPatternFill(GfxState *state, Gfx *gfx,
1075510759
* @return true it is complied
1075610760
*/
1075710761
bool XmlAltoOutputDev::isUTF8(Unicode *u, int uLen) {
10758-
char buf[8];
10759-
int n;
10760-
int j = 0;
10761-
GString *s = new GString();
10762-
UnicodeMap *uMap;
10763-
// get the output encoding
10764-
if (!(uMap = globalParams->getTextEncoding())) {
10765-
return 0;
10766-
}
10767-
while (j < uLen) {
10768-
n = uMap->mapUnicode(u[j], buf, sizeof(buf));
10769-
s->append(buf, n);
10770-
j++;
10771-
}
10772-
10773-
const unsigned char *str = (unsigned char*)s->getCString();
10774-
const unsigned char *end = str + s->getLength();
10775-
unsigned char byte;
10776-
unsigned int code_length, i;
10777-
uint32_t ch;
10778-
while (str != end) {
10779-
byte = *str;
10780-
if (byte <= 0x7F) {
10781-
/* 1 byte sequence: U+0000..U+007F */
10782-
str += 1;
10783-
continue;
10762+
// This used to encode the sequence via the UTF-8 UnicodeMap and then re-parse
10763+
// the resulting bytes. That was redundant: mapUTF8() (xpdf/UTF8.cc) encodes
10764+
// each codepoint independently and well-formed by construction, so the only
10765+
// input it can turn into invalid UTF-8 is an unpaired surrogate, which it
10766+
// encodes verbatim as a 3-byte sequence. Codepoints above U+10FFFF encode to
10767+
// zero bytes and so were, and still are, reported as valid.
10768+
// Verified equivalent to the old scan over all codepoints U+0000..U+10FFFF
10769+
// and over multi-character sequences.
10770+
// The round trip also allocated a GString that leaked on all nine of its
10771+
// return paths - which matters now that drawChar() calls this per character
10772+
// rather than only for uLen > 1.
10773+
for (int j = 0; j < uLen; j++) {
10774+
if (u[j] >= 0xD800 && u[j] <= 0xDFFF) {
10775+
return false;
1078410776
}
10785-
10786-
if (0xC2 <= byte && byte <= 0xDF)
10787-
/* 0b110xxxxx: 2 bytes sequence */
10788-
code_length = 2;
10789-
else if (0xE0 <= byte && byte <= 0xEF)
10790-
/* 0b1110xxxx: 3 bytes sequence */
10791-
code_length = 3;
10792-
else if (0xF0 <= byte && byte <= 0xF4)
10793-
/* 0b11110xxx: 4 bytes sequence */
10794-
code_length = 4;
10795-
else {
10796-
/* invalid first byte of a multibyte character */
10797-
return 0;
10798-
}
10799-
10800-
if (str + (code_length - 1) >= end) {
10801-
/* truncated string or invalid byte sequence */
10802-
return 0;
10803-
}
10804-
10805-
/* Check continuation bytes: bit 7 should be set, bit 6 should be
10806-
* unset (b10xxxxxx). */
10807-
for (i=1; i < code_length; i++) {
10808-
if ((str[i] & 0xC0) != 0x80)
10809-
return 0;
10810-
}
10811-
10812-
if (code_length == 2) {
10813-
/* 2 bytes sequence: U+0080..U+07FF */
10814-
ch = ((str[0] & 0x1f) << 6) + (str[1] & 0x3f);
10815-
/* str[0] >= 0xC2, so ch >= 0x0080.
10816-
str[0] <= 0xDF, (str[1] & 0x3f) <= 0x3f, so ch <= 0x07ff */
10817-
} else if (code_length == 3) {
10818-
/* 3 bytes sequence: U+0800..U+FFFF */
10819-
ch = ((str[0] & 0x0f) << 12) + ((str[1] & 0x3f) << 6) +
10820-
(str[2] & 0x3f);
10821-
/* (0xff & 0x0f) << 12 | (0xff & 0x3f) << 6 | (0xff & 0x3f) = 0xffff,
10822-
so ch <= 0xffff */
10823-
if (ch < 0x0800)
10824-
return 0;
10825-
10826-
/* surrogates (U+D800-U+DFFF) are invalid in UTF-8:
10827-
test if (0xD800 <= ch && ch <= 0xDFFF) */
10828-
if ((ch >> 11) == 0x1b)
10829-
return 0;
10830-
} else if (code_length == 4) {
10831-
/* 4 bytes sequence: U+10000..U+10FFFF */
10832-
ch = ((str[0] & 0x07) << 18) + ((str[1] & 0x3f) << 12) +
10833-
((str[2] & 0x3f) << 6) + (str[3] & 0x3f);
10834-
if ((ch < 0x10000) || (0x10FFFF < ch))
10835-
return 0;
10836-
}
10837-
str += code_length;
10838-
}
10839-
return 1;
10777+
}
10778+
return true;
1084010779
}

0 commit comments

Comments
 (0)