Changeset 14991 in josm for trunk

Timestamp:

2019-04-14T22:17:22+02:00 (5 years ago)

Author:

Don-vip

Message:

fix #17595 - smarter detection of ZWNJ/ZWJ unicode characters

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/validation/tests/TagChecker.java (modified) (5 diffs)
• test/unit/org/openstreetmap/josm/data/validation/tests/TagCheckerTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java

@@ -75 +75 @@
     private static volatile MultiMap<String, String> oftenUsedTags = new MultiMap<>();
 
-    private static final Pattern NON_PRINTING_CONTROL_CHARACTERS = Pattern.compile(
-            "[\\x00-\\x09\\x0B\\x0C\\x0E-\\x1F\\x7F\\u200c-\\u200f\\u202a-\\u202e]");
+    private static final Pattern UNWANTED_NON_PRINTING_CONTROL_CHARACTERS = Pattern.compile(
+            "[\\x00-\\x09\\x0B\\x0C\\x0E-\\x1F\\x7F\\u200e-\\u200f\\u202a-\\u202e]");
 
     /** The TagChecker data */
@@ -378 +378 @@
 
     /**
-     * Checks given string (key or value) if it contains non-printing control characters (either ASCII or Unicode bidi characters)
+     * Checks given string (key or value) if it contains unwanted non-printing control characters (either ASCII or Unicode bidi characters)
      * @param s string to check
      * @return {@code true} if {@code s} contains non-printing control characters
      */
-    private static boolean containsNonPrintingControlCharacter(String s) {
-        return s != null && s.chars().anyMatch(c -> (isAsciiControlChar(c) && !isNewLineChar(c)) || isBidiControlChar(c));
+    static boolean containsUnwantedNonPrintingControlCharacter(String s) {
+        return s != null && !s.isEmpty() && (
+                isJoiningChar(s.charAt(0)) ||
+                isJoiningChar(s.charAt(s.length() - 1)) ||
+                s.chars().anyMatch(c -> (isAsciiControlChar(c) && !isNewLineChar(c)) || isBidiControlChar(c))
+                );
     }
 
@@ -394 +398 @@
     }
 
+    private static boolean isJoiningChar(int c) {
+        return c == 0x200c || c == 0x200d; // ZWNJ, ZWJ
+    }
+
     private static boolean isBidiControlChar(int c) {
-        /* check for range 0x200c to 0x200f (ZWNJ, ZWJ, LRM, RLM) or
+        /* check for range 0x200e to 0x200f (LRM, RLM) or
                            0x202a to 0x202e (LRE, RLE, PDF, LRO, RLO) */
-        return (((c & 0xfffffffc) == 0x200c) || ((c >= 0x202a) && (c <= 0x202e)));
-    }
-
-    static String removeNonPrintingControlCharacters(String s) {
-        return NON_PRINTING_CONTROL_CHARACTERS.matcher(s).replaceAll("");
+        return (c >= 0x200e && c <= 0x200f) || (c >= 0x202a && c <= 0x202e);
+    }
+
+    static String removeUnwantedNonPrintingControlCharacters(String s) {
+        // Remove all unwanted characters
+        String result = UNWANTED_NON_PRINTING_CONTROL_CHARACTERS.matcher(s).replaceAll("");
+        // Remove joining characters located at the beginning of the string
+        while (!result.isEmpty() && isJoiningChar(result.charAt(0))) {
+            result = result.substring(1);
+        }
+        // Remove joining characters located at the end of the string
+        while (!result.isEmpty() && isJoiningChar(result.charAt(result.length() - 1))) {
+            result = result.substring(0, result.length() - 1);
+        }
+        return result;
     }
 
@@ -583 +601 @@
         if (!checkValues || value == null)
             return;
-        if ((containsNonPrintingControlCharacter(value)) && !withErrors.contains(p, "ICV")) {
+        if ((containsUnwantedNonPrintingControlCharacter(value)) && !withErrors.contains(p, "ICV")) {
             errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_VALUE)
                     .message(tr("Tag value contains non-printing character"), s, key)
                     .primitives(p)
-                    .fix(() -> new ChangePropertyCommand(p, key, removeNonPrintingControlCharacters(value)))
+                    .fix(() -> new ChangePropertyCommand(p, key, removeUnwantedNonPrintingControlCharacters(value)))
                     .build());
             withErrors.put(p, "ICV");
@@ -639 +657 @@
         if (!checkKeys || key == null)
             return;
-        if ((containsNonPrintingControlCharacter(key)) && !withErrors.contains(p, "ICK")) {
+        if ((containsUnwantedNonPrintingControlCharacter(key)) && !withErrors.contains(p, "ICK")) {
             errors.add(TestError.builder(this, Severity.WARNING, LOW_CHAR_KEY)
                     .message(tr("Tag key contains non-printing character"), s, key)
                     .primitives(p)
-                    .fix(() -> new ChangePropertyCommand(p, key, removeNonPrintingControlCharacters(key)))
+                    .fix(() -> new ChangePropertyCommand(p, key, removeUnwantedNonPrintingControlCharacters(key)))
                     .build());
             withErrors.put(p, "ICK");

trunk/test/unit/org/openstreetmap/josm/data/validation/tests/TagCheckerTest.java

@@ -9 +9 @@
 import java.util.ArrayList;
 import java.util.List;
-
+import java.util.function.Consumer;
+
+import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
@@ -248 +250 @@
     }
 
-    /**
-     * Unit test of {@link TagChecker#removeNonPrintingControlCharacters}
-     */
-    @Test
-    public void testRemoveUnprintableControlCharacters() {
+    private static void doTestUnwantedNonprintingControlCharacters(String s, Consumer<Boolean> assertionC, String expected) {
+        assertionC.accept(TagChecker.containsUnwantedNonPrintingControlCharacter(s));
+        assertEquals(expected, TagChecker.removeUnwantedNonPrintingControlCharacters(s));
+    }
+
+    private static void doTestUnwantedNonprintingControlCharacters(String s) {
+        doTestUnwantedNonprintingControlCharacters(s, Assert::assertTrue, "");
+    }
+
+    /**
+     * Unit test of {@link TagChecker#containsUnwantedNonPrintingControlCharacter}
+     *            / {@link TagChecker#removeUnwantedNonPrintingControlCharacters}
+     */
+    @Test
+    public void testContainsRemoveUnwantedNonprintingControlCharacters() {
+        // Check empty string is handled
+        doTestUnwantedNonprintingControlCharacters("", Assert::assertFalse, "");
         // Check 65 ASCII control characters are removed, except new lines
         for (char c = 0x0; c < 0x20; c++) {
             if (c != '\r' && c != '\n') {
-                assertTrue(TagChecker.removeNonPrintingControlCharacters(Character.toString(c)).isEmpty());
+                doTestUnwantedNonprintingControlCharacters(Character.toString(c));
             } else {
-                assertFalse(TagChecker.removeNonPrintingControlCharacters(Character.toString(c)).isEmpty());
+                doTestUnwantedNonprintingControlCharacters(Character.toString(c), Assert::assertFalse, Character.toString(c));
             }
         }
-        assertTrue(TagChecker.removeNonPrintingControlCharacters(Character.toString((char) 0x7F)).isEmpty());
-        // Check 9 Unicode bidi control characters are removed
-        for (char c = 0x200c; c <= 0x200f; c++) {
-            assertTrue(TagChecker.removeNonPrintingControlCharacters(Character.toString(c)).isEmpty());
+        doTestUnwantedNonprintingControlCharacters(Character.toString((char) 0x7F));
+        // Check 7 Unicode bidi control characters are removed
+        for (char c = 0x200e; c <= 0x200f; c++) {
+            doTestUnwantedNonprintingControlCharacters(Character.toString(c));
         }
         for (char c = 0x202a; c <= 0x202e; c++) {
-            assertTrue(TagChecker.removeNonPrintingControlCharacters(Character.toString(c)).isEmpty());
+            doTestUnwantedNonprintingControlCharacters(Character.toString(c));
+        }
+        // Check joining characters are removed if located at the beginning or end of the string
+        for (char c = 0x200c; c <= 0x200d; c++) {
+            final String s = Character.toString(c);
+            doTestUnwantedNonprintingControlCharacters(s);
+            doTestUnwantedNonprintingControlCharacters(s + s);
+            doTestUnwantedNonprintingControlCharacters(s + 'a' + s, Assert::assertTrue, "a");
+            final String ok = 'a' + s + 'b';
+            doTestUnwantedNonprintingControlCharacters(ok, Assert::assertFalse, ok);
+            doTestUnwantedNonprintingControlCharacters(s + ok, Assert::assertTrue, ok);
+            doTestUnwantedNonprintingControlCharacters(ok + s, Assert::assertTrue, ok);
+            doTestUnwantedNonprintingControlCharacters(s + ok + s, Assert::assertTrue, ok);
         }
     }