Revert #1345, clarify comments and add tests (#1348)

java-native-access · May 6, 2021 · f89905a · f89905a
1 parent 1fb4d80
commit f89905a
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 31 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -14,7 +14,7 @@ Features
 
 Bug Fixes
 ---------
-* [#1343](https://github.com/java-native-access/jna/issues/1343), [#1345](https://github.com/java-native-access/jna/issues/1345): `c.s.j.p.mac.CoreFoundation.CFStringRef#stringValue` buffer needs space for 4 UTF8 bytes plus a null byte - [@dbwiddis](https://github.com/dbwiddis).
+* [#1343](https://github.com/java-native-access/jna/issues/1343): `c.s.j.p.mac.CoreFoundation.CFStringRef#stringValue` buffer needs space for a null byte - [@dbwiddis](https://github.com/dbwiddis).
 
 Release 5.8.0
 =============

diff --git a/contrib/platform/src/com/sun/jna/platform/mac/CoreFoundation.java b/contrib/platform/src/com/sun/jna/platform/mac/CoreFoundation.java
@@ -487,20 +487,21 @@ public static CFStringRef createCFString(String s) {
          *         failed.
          */
         public String stringValue() {
-            // Get number of characters
-            long length = INSTANCE.CFStringGetLength(this).longValue();
-            if (length == 0) {
+            // Get number of characters (UTF-16 code pairs)
+            // Code points > 0xffff will have 2 characters per Unicode character
+            CFIndex length = INSTANCE.CFStringGetLength(this);
+            if (length.longValue() == 0) {
                 return "";
             }
             // Calculate maximum possible size in UTF8 bytes
-            // the CFStringGetMaximumSizeForEncoding function incorrectly returns 3 bytes
-            // per character so we'll use 4 bytes plus a null byte
-            if (length > (Long.MAX_VALUE - 1) / 4) {
+            // This will be 3 x length
+            CFIndex maxSize = INSTANCE.CFStringGetMaximumSizeForEncoding(length, kCFStringEncodingUTF8);
+            if (maxSize.intValue() == kCFNotFound) {
                 throw new StringIndexOutOfBoundsException("CFString maximum number of bytes exceeds LONG_MAX.");
             }
-            long bufSize = 4 * length + 1;
-            CFIndex maxSize = new CFIndex(bufSize);
-            Memory buf = new Memory(bufSize);
+            // Increment size by 1 for a null byte
+            maxSize.setValue(maxSize.longValue() + 1);
+            Memory buf = new Memory(maxSize.longValue());
             if (0 != INSTANCE.CFStringGetCString(this, buf, maxSize, kCFStringEncodingUTF8)) {
                 return buf.getString(0, "UTF8");
             }

diff --git a/contrib/platform/test/com/sun/jna/platform/mac/CoreFoundationTest.java b/contrib/platform/test/com/sun/jna/platform/mac/CoreFoundationTest.java
@@ -63,27 +63,37 @@ public class CoreFoundationTest {
 
     @Test
     public void testCFStringRef() throws UnsupportedEncodingException {
-        // Create a unicode string of a single 4-byte character
-        byte[] smileEmoji = { (byte) 0xF0, (byte) 0x9F, (byte) 0x98, (byte) 0x83 };
-        String utf8Str = new String(smileEmoji, StandardCharsets.UTF_8);
-        CFStringRef cfStr = CFStringRef.createCFString(utf8Str);
-        assertEquals(utf8Str.length(), CF.CFStringGetLength(cfStr).intValue());
-        assertEquals(utf8Str, cfStr.stringValue());
-        assertEquals(CoreFoundation.STRING_TYPE_ID, cfStr.getTypeID());
-
-        byte[] utf8Arr = utf8Str.getBytes("UTF-8");
-        Memory mem = new Memory(utf8Arr.length + 1);
-        mem.clear();
-        assertNotEquals(0,
-                CF.CFStringGetCString(cfStr, mem, new CFIndex(mem.size()), CoreFoundation.kCFStringEncodingUTF8));
-        byte[] utf8Bytes = mem.getByteArray(0, (int) mem.size() - 1);
-        assertArrayEquals(utf8Arr, utf8Bytes);
-        // Essentially a toString, can't rely on format but should contain the string
-        CFStringRef desc = CF.CFCopyDescription(cfStr);
-        assertTrue(desc.stringValue().contains(utf8Str));
-
-        desc.release();
-        cfStr.release();
+        // Generate strings with different UTF8 byte lengths
+        byte[] pound = { (byte) 0xc2, (byte) 0xa3 };
+        byte[] euro = { (byte) 0xe2, (byte) 0x82, (byte) 0xac };
+        byte[] smileEmoji = { (byte) 0xf0, (byte) 0x9f, (byte) 0x98, (byte) 0x83 };
+        String[] testStrings = new String[4];
+        testStrings[0] = "ascii";
+        testStrings[1] = new String(pound, StandardCharsets.UTF_8);
+        testStrings[2] = new String(euro, StandardCharsets.UTF_8);
+        testStrings[3] = new String(smileEmoji, StandardCharsets.UTF_8);
+        for (String utf8Str : testStrings) {
+            CFStringRef cfStr = CFStringRef.createCFString(utf8Str);
+            // Length matches length of char array
+            // 2 for code points > 0xffff, 1 otherwise
+            assertEquals(utf8Str.length(), CF.CFStringGetLength(cfStr).intValue());
+            assertEquals(utf8Str, cfStr.stringValue());
+            assertEquals(CoreFoundation.STRING_TYPE_ID, cfStr.getTypeID());
+
+            byte[] utf8Arr = utf8Str.getBytes("UTF-8");
+            Memory mem = new Memory(utf8Arr.length + 1);
+            mem.clear();
+            assertNotEquals(0,
+                    CF.CFStringGetCString(cfStr, mem, new CFIndex(mem.size()), CoreFoundation.kCFStringEncodingUTF8));
+            byte[] utf8Bytes = mem.getByteArray(0, (int) mem.size() - 1);
+            assertArrayEquals(utf8Arr, utf8Bytes);
+            // Essentially a toString, can't rely on format but should contain the string
+            CFStringRef desc = CF.CFCopyDescription(cfStr);
+            assertTrue(desc.stringValue().contains(utf8Str));
+
+            desc.release();
+            cfStr.release();
+        }
 
         CFStringRef cfEmpty = CFStringRef.createCFString("");
         assertTrue(cfEmpty.stringValue().equals(""));