From c2e9c511cbc4bd374d3a0680c58da60043ef21c3 Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Sun, 14 Apr 2013 16:12:08 +0900 Subject: [PATCH] Fix Binary dict tests There are two problems here. The first one is the tests would send an invalid unicode character. Although we could want dicttool to handle this more gracefully, it's fine for now. The second problem is much more serious. If a node has more than 128 children, then the java code will crash trying to read the dictionary back because of a bug that this change fixes. In theory, it's possible that happens when we try to load the user history dictionary back from the disk - native code is not affected so there is no other point that may cause a problem. In the practice, that means you'd need to have 129 words with a common prefix (including empty string) but all different after this. It's almost impossible with Google Keyboard since there are only so many keys on the keyboard that you can make a word out of, and then again you'd have to do it repeatedly until it actually enters the user history dictionary, wait for it to get saved on the disk. The bad news is, if you manage to get this far, the keyboard will crash every time and won't be able to get up until you clear data for the package. The good news is, the dictionary itself is not corrupted and only the reading code is wrong. So updating to a newer version would actually even recover from this situation. All in all, considering how almost-impossible this is to trigger, I don't think even a single user actually did hit this bug. Bug: 8583091 Change-Id: Iabb2a7f47cbd9ed3193d2a3487318d280753e071 --- .../inputmethod/latin/makedict/BinaryDictInputOutput.java | 8 ++++---- .../inputmethod/latin/makedict/BinaryDictIOTests.java | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/makedict/BinaryDictInputOutput.java b/java/src/com/android/inputmethod/latin/makedict/BinaryDictInputOutput.java index 58ec1e83f..467f6a053 100644 --- a/java/src/com/android/inputmethod/latin/makedict/BinaryDictInputOutput.java +++ b/java/src/com/android/inputmethod/latin/makedict/BinaryDictInputOutput.java @@ -1467,8 +1467,8 @@ public final class BinaryDictInputOutput { if (null == last) continue; builder.append(new String(last.mCharacters, 0, last.mCharacters.length)); buffer.position(last.mChildrenAddress + headerSize); - groupOffset = last.mChildrenAddress + 1; - i = buffer.readUnsignedByte(); + i = readCharGroupCount(buffer); + groupOffset = last.mChildrenAddress + getGroupCountSize(i); last = null; continue; } @@ -1477,8 +1477,8 @@ public final class BinaryDictInputOutput { if (0 == i && hasChildrenAddress(last.mChildrenAddress)) { builder.append(new String(last.mCharacters, 0, last.mCharacters.length)); buffer.position(last.mChildrenAddress + headerSize); - groupOffset = last.mChildrenAddress + 1; - i = buffer.readUnsignedByte(); + i = readCharGroupCount(buffer); + groupOffset = last.mChildrenAddress + getGroupCountSize(i); last = null; continue; } diff --git a/tests/src/com/android/inputmethod/latin/makedict/BinaryDictIOTests.java b/tests/src/com/android/inputmethod/latin/makedict/BinaryDictIOTests.java index bd8729203..89913fa0a 100644 --- a/tests/src/com/android/inputmethod/latin/makedict/BinaryDictIOTests.java +++ b/tests/src/com/android/inputmethod/latin/makedict/BinaryDictIOTests.java @@ -137,7 +137,10 @@ public class BinaryDictIOTests extends AndroidTestCase { if (r < 0) continue; // Don't insert 0~20, but insert any other code point. // Code points are in the range 0~0x10FFFF. - builder.appendCodePoint((int)(20 + r % (0x10FFFF - 20))); + final int candidateCodePoint = (int)(20 + r % (0x10FFFF - 20)); + // Code points between 0xD800 and 0xDFFF are not valid. + if (candidateCodePoint >= 0xD800 && candidateCodePoint <= 0xDFFF) continue; + builder.appendCodePoint(candidadeCodePoint); --count; } return builder.toString();