diff --git a/fop-core/src/main/java/org/apache/fop/complexscripts/bidi/UnicodeBidiAlgorithm.java b/fop-core/src/main/java/org/apache/fop/complexscripts/bidi/UnicodeBidiAlgorithm.java index 9857ae8f719..0096476d6d9 100644 --- a/fop-core/src/main/java/org/apache/fop/complexscripts/bidi/UnicodeBidiAlgorithm.java +++ b/fop-core/src/main/java/org/apache/fop/complexscripts/bidi/UnicodeBidiAlgorithm.java @@ -75,7 +75,17 @@ public static int[] resolveLevels(CharSequence cs, Direction defaultLevel) { * @param levels array to receive levels, one for each character in chars array */ public static int[] resolveLevels(int[] chars, int defaultLevel, int[] levels) { - return resolveLevels(chars, getClasses(chars), defaultLevel, levels, false); + resolveLevels(chars, getClasses(chars), defaultLevel, levels, false); + // The placeholder that stands for the low surrogate of a pair takes the level of the + // character it belongs to, as resolveLevels(CharSequence, Direction) documents. With the + // placeholder classed as its character (getClasses) the rules already agree; this holds + // the promise whatever a rule does with a repeated class. + for (int i = 1, n = chars.length; i < n; i++) { + if (chars [ i ] < 0) { + levels [ i ] = levels [ i - 1 ]; + } + } + return levels; } /** @@ -618,6 +628,13 @@ private static int[] getClasses(int[] chars) { int ch = chars [ i ]; if (ch >= 0) { bc = BidiClass.getBidiClass(chars [ i ]); + } else if (i > 0) { + // The placeholder for a low surrogate takes the class of the character it belongs + // to, so that the rules see the pair as the one character it is. As a class of its + // own it ended a run of neutrals in rule N1, so a neutral outside the BMP inside + // right-to-left text fell to the embedding direction where one in the BMP takes the + // text's. + bc = classes [ i - 1 ]; } else { bc = SURROGATE; } diff --git a/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java b/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java index 4b48cd43304..d4b998f7819 100644 --- a/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java +++ b/fop-core/src/main/java/org/apache/fop/layoutmgr/inline/TextLayoutManager.java @@ -779,6 +779,7 @@ public List getNextKnuthElements(final LayoutContext context, fin boolean inWord = false; boolean inWhitespace = false; char ch = 0; + char prevChar = 0; int level = -1; int prevLevel = -1; boolean retainControls = false; @@ -819,8 +820,9 @@ public List getNextKnuthElements(final LayoutContext context, fin boolean processWord = breakOpportunity || GlyphMapping.isSpace(ch) || CharUtilities.isExplicitBreak(ch) - || ((prevLevel != -1) && (level != prevLevel)); - if (!processWord && foText.getCommonFont().getFontSelectionStrategy() == EN_CHARACTER_BY_CHARACTER) { + || ((prevLevel != -1) && (level != prevLevel) && !Character.isHighSurrogate(prevChar)); + if (!processWord && !Character.isHighSurrogate(prevChar) + && foText.getCommonFont().getFontSelectionStrategy() == EN_CHARACTER_BY_CHARACTER) { if (lastFont == null || lastFontPos != nextStart - 1) { lastFont = FontSelector.selectFontForCharactersInText( foText, nextStart - 1, nextStart, foText, this); @@ -887,6 +889,7 @@ public List getNextKnuthElements(final LayoutContext context, fin inWhitespace = ch == CharUtilities.SPACE && foText.getWhitespaceTreatment() != Constants.EN_PRESERVE; prevLevel = level; + prevChar = ch; nextStart++; } diff --git a/fop-core/src/main/java/org/apache/fop/util/CharUtilities.java b/fop-core/src/main/java/org/apache/fop/util/CharUtilities.java index 4be495209b1..27e2291ba79 100644 --- a/fop-core/src/main/java/org/apache/fop/util/CharUtilities.java +++ b/fop-core/src/main/java/org/apache/fop/util/CharUtilities.java @@ -409,7 +409,7 @@ public static boolean containsSurrogatePairAt(CharSequence chars, int index) { char ch = chars.charAt(index); if (Character.isHighSurrogate(ch)) { - if ((index + 1) > chars.length()) { + if ((index + 1) >= chars.length()) { throw new IllegalArgumentException( "ill-formed UTF-16 sequence, contains isolated high surrogate at end of sequence"); } diff --git a/fop-core/src/test/java/org/apache/fop/complexscripts/bidi/SurrogatePairLevelsTestCase.java b/fop-core/src/test/java/org/apache/fop/complexscripts/bidi/SurrogatePairLevelsTestCase.java new file mode 100644 index 00000000000..a0c7dcd0ac5 --- /dev/null +++ b/fop-core/src/test/java/org/apache/fop/complexscripts/bidi/SurrogatePairLevelsTestCase.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* $Id$ */ + +package org.apache.fop.complexscripts.bidi; + +import org.junit.Test; +import static org.junit.Assert.assertArrayEquals; + +import org.apache.fop.traits.Direction; + +/** + * Both UTF-16 units of a supplementary-plane character must resolve to one bidi level, as + * {@link UnicodeBidiAlgorithm#resolveLevels(CharSequence, Direction)} documents. U+10826 is + * a Cypriot syllable, a right-to-left script outside the BMP (FOP-2918). + */ +public class SurrogatePairLevelsTestCase { + + private static final String CYPRIOT = "ЁРаж"; + + /** U+1F300 CYCLONE, a neutral (ON) outside the BMP that FOP's bidi class table knows as one. */ + private static final String CYCLONE = "\uD83C\uDF00"; + + private static final String SHALOM = "\u05E9\u05DC\u05D5\u05DD"; + + private static final String OLAM = "\u05E2\u05D5\u05DC\u05DD"; + + @Test + public void testPairAlone() { + assertArrayEquals(new int[] {1, 1}, UnicodeBidiAlgorithm.resolveLevels(CYPRIOT, Direction.LR)); + } + + @Test + public void testPairBetweenLatinLetters() { + assertArrayEquals(new int[] {0, 1, 1, 0}, + UnicodeBidiAlgorithm.resolveLevels("a" + CYPRIOT + "b", Direction.LR)); + } + + @Test + public void testTwoPairs() { + assertArrayEquals(new int[] {1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels(CYPRIOT + CYPRIOT, Direction.LR)); + } + + /** + * A neutral outside the BMP inside right-to-left text resolves as a neutral in the BMP does + * (U+263A here): it takes the text's direction by rule N1, so the run is not cut in two. The + * placeholder for the low surrogate, as a class of its own, ended the run of neutrals, and the + * pair fell to the embedding direction; copying the level after resolution cannot mend that. + */ + @Test + public void testNeutralPairInsideRightToLeftText() { + assertArrayEquals(new int[] {1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels(SHALOM + "\u263A " + OLAM, Direction.LR)); + assertArrayEquals(new int[] {1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels(SHALOM + CYCLONE + " " + OLAM, Direction.LR)); + assertArrayEquals(new int[] {1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels(SHALOM + " " + CYCLONE + " " + OLAM, Direction.LR)); + } + + /** Between left-to-right and right-to-left text the same neutral takes the embedding direction (N2). */ + @Test + public void testNeutralPairBetweenDirections() { + assertArrayEquals(new int[] {0, 0, 0, 0, 0, 1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels("ab" + CYCLONE + " " + OLAM, Direction.LR)); + assertArrayEquals(new int[] {2, 2, 1, 1, 1, 1, 1, 1, 1}, + UnicodeBidiAlgorithm.resolveLevels("ab" + CYCLONE + " " + OLAM, Direction.RL)); + } +} diff --git a/fop-core/src/test/java/org/apache/fop/render/pdf/PDFEncodingTestCase.java b/fop-core/src/test/java/org/apache/fop/render/pdf/PDFEncodingTestCase.java index eb2024c27d6..3b13951e96f 100644 --- a/fop-core/src/test/java/org/apache/fop/render/pdf/PDFEncodingTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/render/pdf/PDFEncodingTestCase.java @@ -117,6 +117,26 @@ public void testPDFEncodingWithNonBMPFont() throws Exception { runTest("test-custom-non-bmp-font.fo", testPatterns); } + /** + * Test a non-BMP character under per-character font selection. The selected font changes at the + * surrogate pair, which must not end the word between the high surrogate and its low surrogate. + * Before that was fixed, layout raised IllegalArgumentException and no PDF was produced. + * + * @throws Exception + * checkstyle wants a comment here, even a silly one + */ + @Test + public void testPDFEncodingWithNonBMPFontCharacterByCharacter() throws Exception { + + final String[] testPatterns = { + TEST_MARKER + "1", "\uD800\uDF00", + TEST_MARKER + "2", "\uD800\uDF00", + TEST_MARKER + "3", "\uD800\uDF00", + }; + + runTest("test-non-bmp-character-by-character.fo", testPatterns); + } + /** Test encoding using specified input file and test patterns array */ private void runTest(String inputFile, String[] testPatterns) throws Exception { diff --git a/fop-core/src/test/java/org/apache/fop/util/CharUtilitiesTestCase.java b/fop-core/src/test/java/org/apache/fop/util/CharUtilitiesTestCase.java index b10ae8aea56..82b8491cca1 100644 --- a/fop-core/src/test/java/org/apache/fop/util/CharUtilitiesTestCase.java +++ b/fop-core/src/test/java/org/apache/fop/util/CharUtilitiesTestCase.java @@ -69,4 +69,14 @@ public void testContainsSurrogatePairAtWithMalformedUTF8Sequence() { CharUtilities.containsSurrogatePairAt(malformedUTF8Sequence, 3); } + + /** A high surrogate as the last character is ill-formed UTF-16, so the documented + * IllegalArgumentException is what callers must see - not the IndexOutOfBoundsException + * that reading one past the end would raise. */ + @Test(expected = IllegalArgumentException.class) + public void testContainsSurrogatePairAtWithIsolatedHighSurrogateAtEndOfSequence() { + String isolatedHighSurrogateAtEnd = "012\uD83D"; + + CharUtilities.containsSurrogatePairAt(isolatedHighSurrogateAtEnd, 3); + } } diff --git a/fop/test/layoutengine/standard-testcases/wordbreak_surrogates.xml b/fop/test/layoutengine/standard-testcases/wordbreak_surrogates.xml new file mode 100644 index 00000000000..7b6c33a615a --- /dev/null +++ b/fop/test/layoutengine/standard-testcases/wordbreak_surrogates.xml @@ -0,0 +1,47 @@ + + + + + +

+ This test checks that a right to left surrogate pair does not get + word broken in the middle causing an exception. This is for issue FOP-2918 + java.lang.IllegalArgumentException: ill-formed UTF-16 sequence, + contains isolated high surrogate at end of sequence. +

+ + + + + + + + + + + + 𐠦 + + + + + + + + + diff --git a/fop/test/xml/pdf-encoding/test-non-bmp-character-by-character.fo b/fop/test/xml/pdf-encoding/test-non-bmp-character-by-character.fo new file mode 100644 index 00000000000..0e026987cae --- /dev/null +++ b/fop/test/xml/pdf-encoding/test-non-bmp-character-by-character.fo @@ -0,0 +1,40 @@ + + + + + + + + + + + + + + + PDFE_TEST_MARK_1: pair between words 𐌀 here + PDFE_TEST_MARK_2: pair last in the block 𐌀 + PDFE_TEST_MARK_3: consecutive pairs 𐌀𐌀𐌀 + + +