Skip to content

ICU-23513 Fixed PDI overrides in bidi #4205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roozbehp wants to merge 1 commit into
base: main
Choose a base branch
from
+91 −9
Open

ICU-23513 Fixed PDI overrides in bidi #4205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions icu4c/source/common/ubidi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,10 @@ bracketProcessChar(BracketData *bd, int32_t position) {
UBiDiLevel level;
dirProps=bd->pBiDi->dirProps;
dirProp=dirProps[position];
/* Preserve the stored PDI/WS identity for X10 and L1. */
if(dirProp==PDIL || dirProp==WSL) dirProp=L;
else if(dirProp==PDIR || dirProp==WSR) dirProp=R;

if(dirProp==ON) {
char16_t c, match;
int32_t idx;
Expand Down Expand Up @@ -1289,6 +1293,14 @@ resolveExplicitLevels(UBiDi *pBiDi, UErrorCode *pErrorCode) {
flags|=(DIRPROP_FLAG(ON)|DIRPROP_FLAG_LR(embeddingLevel));
previousLevel=embeddingLevel;
levels[i]=NO_OVERRIDE(embeddingLevel);
if(embeddingLevel&UBIDI_LEVEL_OVERRIDE) {
/* X6a: apply the override, retaining the PDI's X10/L1 identity. */
dirProps[i]=static_cast<DirProp>((dirProps[i]==PDI ? PDIL : WSL)+(embeddingLevel&1));
flags|=DIRPROP_FLAG(dirProps[i]);
if(!bracketProcessChar(&bracketData, i))
return static_cast<UBiDiDirection>(-1);
}

break;
case B:
flags|=DIRPROP_FLAG(B);
Expand Down Expand Up @@ -1841,7 +1853,7 @@ setLevelsOutsideIsolates(UBiDi *pBiDi, int32_t start, int32_t limit, UBiDiLevel
int32_t isolateCount=0, k;
for(k=start; k<limit; k++) {
dirProp=dirProps[k];
if(dirProp==PDI)
if(DIRPROP_FLAG(dirProp)&MASK_PDI)
isolateCount--;
if(isolateCount==0)
levels[k]=level;
Expand Down Expand Up @@ -2172,7 +2184,7 @@ resolveImplicitLevels(UBiDi *pBiDi,
/* The isolates[] entries contain enough information to
resume the bidi algorithm in the same state as it was
when it was interrupted by an isolate sequence. */
if(dirProps[start]==PDI && pBiDi->isolateCount >= 0) {
if((DIRPROP_FLAG(dirProps[start])&MASK_PDI) && pBiDi->isolateCount >= 0) {
levState.startON=pBiDi->isolates[pBiDi->isolateCount].startON;
start1=pBiDi->isolates[pBiDi->isolateCount].start1;
stateImp=pBiDi->isolates[pBiDi->isolateCount].stateImp;
Expand Down Expand Up @@ -2228,6 +2240,8 @@ resolveImplicitLevels(UBiDi *pBiDi,
}
}
}
if(prop==PDIL || prop==WSL) prop=L;
else if(prop==PDIR || prop==WSR) prop=R;
gprop=groupProp[prop];
}
oldStateImp=stateImp;
Expand Down
9 changes: 7 additions & 2 deletions icu4c/source/common/ubidiimp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ enum {
PDI=U_POP_DIRECTIONAL_ISOLATE, /* 22 */
ENL, /* EN after W7 */ /* 23 */
ENR, /* EN not subject to W7 */ /* 24 */
PDIL, /* matched PDI overridden to L */ /* 25 */
PDIR, /* matched PDI overridden to R */ /* 26 */
WSL, /* unmatched/overflow PDI overridden to L */ /* 27 */
WSR, /* unmatched/overflow PDI overridden to R */ /* 28 */
dirPropCount
};

Expand All @@ -105,15 +109,16 @@ enum {
#define MASK_EXPLICIT (DIRPROP_FLAG(LRE)|DIRPROP_FLAG(LRO)|DIRPROP_FLAG(RLE)|DIRPROP_FLAG(RLO)|DIRPROP_FLAG(PDF))

/* explicit isolate codes */
#define MASK_ISO (DIRPROP_FLAG(LRI)|DIRPROP_FLAG(RLI)|DIRPROP_FLAG(FSI)|DIRPROP_FLAG(PDI))
#define MASK_PDI (DIRPROP_FLAG(PDI)|DIRPROP_FLAG(PDIL)|DIRPROP_FLAG(PDIR))
#define MASK_ISO (DIRPROP_FLAG(LRI)|DIRPROP_FLAG(RLI)|DIRPROP_FLAG(FSI)|MASK_PDI)

#define MASK_BN_EXPLICIT (DIRPROP_FLAG(BN)|MASK_EXPLICIT)

/* paragraph and segment separators */
#define MASK_B_S (DIRPROP_FLAG(B)|DIRPROP_FLAG(S))

/* all types that are counted as White Space or Neutral in some steps */
#define MASK_WS (MASK_B_S|DIRPROP_FLAG(WS)|MASK_BN_EXPLICIT|MASK_ISO)
#define MASK_WS (MASK_B_S|DIRPROP_FLAG(WS)|DIRPROP_FLAG(WSL)|DIRPROP_FLAG(WSR)|MASK_BN_EXPLICIT|MASK_ISO)

/* types that are neutrals or could becomes neutrals in (Wn) */
#define MASK_POSSIBLE_N (DIRPROP_FLAG(ON)|DIRPROP_FLAG(CS)|DIRPROP_FLAG(ES)|DIRPROP_FLAG(ET)|MASK_WS)
Expand Down
28 changes: 28 additions & 0 deletions icu4c/source/test/cintltst/cbiditst.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ static void doTailTest(void);

static void testBracketOverflow(void);
static void TestExplicitLevel0(void);
static void testUnmatchedPDIOverride(void);
static void testUBidiWriteReorderedBufferOverflow(void);
static void testUBidiWriteReorderedUndefinedShift(void);
static void testUBidiWriteReorderedReverseMirrorCombining(void);
Expand Down Expand Up @@ -147,6 +148,7 @@ addComplexTest(TestNode** root) {
addTest(root, testContext, "complex/bidi/testContext");
addTest(root, testBracketOverflow, "complex/bidi/TestBracketOverflow");
addTest(root, TestExplicitLevel0, "complex/bidi/TestExplicitLevel0");
addTest(root, testUnmatchedPDIOverride, "complex/bidi/TestUnmatchedPDIOverride");
addTest(root, testUBidiWriteReorderedBufferOverflow, "complex/bidi/writeReorderedBufferOverflow");
addTest(root, testUBidiWriteReorderedUndefinedShift, "complex/bidi/writeReorderedUndefinedShift");
addTest(root, testUBidiWriteReorderedReverseMirrorCombining, "complex/bidi/writeReorderedReverseMirrorCombining");
Expand Down Expand Up @@ -5124,6 +5126,32 @@ static void TestExplicitLevel0(void) {
ubidi_close(bidi);
}

static void
testUnmatchedPDIOverride(void) {
/* ICU-23513 (J1): X6a applies the active RLO to an unmatched PDI. */
static const UChar text[] = { 0x202A, 0x0061, 0x202C, 0x202E, 0x2069, 0x202A, 0x0062 };
UErrorCode status = U_ZERO_ERROR;
UChar dest[3];
UBiDi *bidi = ubidi_openSized(UPRV_LENGTHOF(text), 0, &status);
if (!assertSuccess("ubidi_openSized", &status)) {
return;
}
ubidi_setPara(bidi, text, UPRV_LENGTHOF(text), UBIDI_LTR, NULL, &status);
if (assertSuccess("ubidi_setPara", &status)) {
/* Expected levels: x 2 x x 1 x 2; X9-removed controls are not checked. */
assertIntEquals("level of a", 2, ubidi_getLevelAt(bidi, 1));
assertIntEquals("level of PDI", 1, ubidi_getLevelAt(bidi, 4));
assertIntEquals("level of b", 2, ubidi_getLevelAt(bidi, 6));
int32_t length = ubidi_writeReordered(bidi, dest, UPRV_LENGTHOF(dest),
UBIDI_REMOVE_BIDI_CONTROLS, &status);
if (assertSuccess("ubidi_writeReordered", &status) &&
assertIntEquals("reordered length", 2, length)) {
assertUEquals("visual order", u"ba", dest);
}
}
ubidi_close(bidi);
}

static void
testUBidiWriteReverseOverflow(void) {
UErrorCode status = U_ZERO_ERROR;
Expand Down
33 changes: 28 additions & 5 deletions icu4j/main/core/src/main/java/com/ibm/icu/text/Bidi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,10 @@ static class Isolate {
static final byte PDI = UCharacterDirection.POP_DIRECTIONAL_ISOLATE; /* 22 */
static final byte ENL = PDI + 1; /* EN after W7 */ /* 23 */
static final byte ENR = ENL + 1; /* EN not subject to W7 */ /* 24 */
static final byte PDIL = ENR + 1; /* matched PDI overridden to L */ /* 25 */
static final byte PDIR = PDIL + 1; /* matched PDI overridden to R */ /* 26 */
static final byte WSL = PDIR + 1; /* unmatched/overflow PDI overridden to L */ /* 27 */
static final byte WSR = WSL + 1; /* unmatched/overflow PDI overridden to R */ /* 28 */

/**
* Value returned by <code>BidiClassifier</code> when there is no need to override the standard
Expand Down Expand Up @@ -1192,14 +1196,20 @@ static final byte NoOverride(byte level) {
static final int MASK_BN_EXPLICIT = DirPropFlag(BN) | MASK_EXPLICIT;

/* explicit isolate codes */
static final int MASK_ISO =
DirPropFlag(LRI) | DirPropFlag(RLI) | DirPropFlag(FSI) | DirPropFlag(PDI);
static final int MASK_PDI = DirPropFlag(PDI) | DirPropFlag(PDIL) | DirPropFlag(PDIR);
static final int MASK_ISO = DirPropFlag(LRI) | DirPropFlag(RLI) | DirPropFlag(FSI) | MASK_PDI;

/* paragraph and segment separators */
static final int MASK_B_S = DirPropFlag(B) | DirPropFlag(S);

/* all types that are counted as White Space or Neutral in some steps */
static final int MASK_WS = MASK_B_S | DirPropFlag(WS) | MASK_BN_EXPLICIT | MASK_ISO;
static final int MASK_WS =
MASK_B_S
| DirPropFlag(WS)
| DirPropFlag(WSL)
| DirPropFlag(WSR)
| MASK_BN_EXPLICIT
| MASK_ISO;

/* types that are neutrals or could becomes neutrals in (Wn) */
static final int MASK_POSSIBLE_N =
Expand Down Expand Up @@ -2163,6 +2173,10 @@ private void bracketProcessChar(BracketData bd, int position) {
byte dirProp, newProp;
byte level;
dirProp = dirProps[position];
/* Preserve the stored PDI/WS identity for X10 and L1. */
if (dirProp == PDIL || dirProp == WSL) dirProp = L;
else if (dirProp == PDIR || dirProp == WSR) dirProp = R;

if (dirProp == ON) {
char c, match;
int idx;
Expand Down Expand Up @@ -2534,6 +2548,13 @@ private byte resolveExplicitLevels() {
flags |= DirPropFlag(ON) | DirPropFlagLR(embeddingLevel);
previousLevel = embeddingLevel;
levels[i] = NoOverride(embeddingLevel);
if ((embeddingLevel & LEVEL_OVERRIDE) != 0) {
/* X6a: apply the override, retaining the PDI's X10/L1 identity. */
dirProps[i] =
(byte) ((dirProps[i] == PDI ? PDIL : WSL) + (embeddingLevel & 1));
flags |= DirPropFlag(dirProps[i]);
bracketProcessChar(bracketData, i);
}
break;
case B:
flags |= DirPropFlag(B);
Expand Down Expand Up @@ -3120,7 +3141,7 @@ private void setLevelsOutsideIsolates(int start, int limit, byte level) {
int isolateCount = 0, k;
for (k = start; k < limit; k++) {
dirProp = dirProps[k];
if (dirProp == PDI) isolateCount--;
if ((DirPropFlag(dirProp) & MASK_PDI) != 0) isolateCount--;
if (isolateCount == 0) levels[k] = level;
if (dirProp == LRI || dirProp == RLI) isolateCount++;
}
Expand Down Expand Up @@ -3424,7 +3445,7 @@ private void resolveImplicitLevels(int start, int limit, short sor, short eor) {
/* The isolates[] entries contain enough information to
resume the bidi algorithm in the same state as it was
when it was interrupted by an isolate sequence. */
if (dirProps[start] == PDI) {
if ((DirPropFlag(dirProps[start]) & MASK_PDI) != 0) {
levState.startON = isolates[isolateCount].startON;
start1 = isolates[isolateCount].start1;
stateImp = isolates[isolateCount].stateImp;
Expand Down Expand Up @@ -3479,6 +3500,8 @@ private void resolveImplicitLevels(int start, int limit, short sor, short eor) {
}
}
}
if (prop == PDIL || prop == WSL) prop = L;
else if (prop == PDIR || prop == WSR) prop = R;
gprop = groupProp[prop];
}
oldStateImp = stateImp;
Expand Down
12 changes: 12 additions & 0 deletions icu4j/main/core/src/test/java/com/ibm/icu/dev/test/bidi/TestBidi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,18 @@ public void testExplicitLevel0() {
assertEquals("java.text resolved level at 1", 1, jb.getLevelAt(1));
}

@Test
public void testUnmatchedPDIOverride() {
// ICU-23513 (J1): X6a applies the active RLO to an unmatched PDI.
Bidi bidi = new Bidi();
bidi.setPara("\u202Aa\u202C\u202E\u2069\u202Ab", Bidi.LTR, null);
// Expected levels: x 2 x x 1 x 2; X9-removed controls are not checked.
assertEquals("level of a", 2, bidi.getLevelAt(1));
assertEquals("level of PDI", 1, bidi.getLevelAt(4));
assertEquals("level of b", 2, bidi.getLevelAt(6));
assertEquals("visual order", "ba", bidi.writeReordered(Bidi.REMOVE_BIDI_CONTROLS));
}

@Test
public void testWriteReorderedUndefinedShift() {
Bidi bidi = new Bidi();
Expand Down
Loading