From 5bb806a057122b160dd8771f8f444dbcf02a4e7e Mon Sep 17 00:00:00 2001 From: Johann Werner Date: Thu, 12 Mar 2015 10:22:50 +0100 Subject: [PATCH] don't pass null to constructor support edge case where param source for ERXStringUtilities.safeIdentifierName is null which would throw a NPE --- .../foundation/ERXStringUtilities.java | 12 +++---- .../foundation/ERXStringUtilitiesTest.java | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java index bf2a0a6d75b..02d3a1eee07 100644 --- a/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java +++ b/Frameworks/Core/ERExtensions/Sources/er/extensions/foundation/ERXStringUtilities.java @@ -2053,15 +2053,13 @@ public static boolean regionMatches(StringBuffer str, int toffset, String other, */ public static String safeIdentifierName(String source, String prefix, char replacement) { - StringBuilder b; + StringBuilder b = new StringBuilder(); // Add prefix if source does not start with valid character - if (source == null || source.length() == 0 || Character.isJavaIdentifierStart(source.charAt(0))) { - b = new StringBuilder(source); - } else { - b = new StringBuilder(prefix); - b.append(source); + if (source == null || source.length() == 0 || !Character.isJavaIdentifierStart(source.charAt(0))) { + b.append(prefix); } - + b.append(source); + for (int i = 0; i < b.length(); i++) { char c = b.charAt(i); if ( ! Character.isJavaIdentifierPart(c)) { diff --git a/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java b/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java index fdf167d2342..0876cd6b9cc 100644 --- a/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java +++ b/Tests/ERXTest/Sources/er/extensions/foundation/ERXStringUtilitiesTest.java @@ -1,6 +1,7 @@ package er.extensions.foundation; import static org.junit.Assert.assertEquals; +import junit.framework.Assert; import junit.framework.JUnit4TestAdapter; import org.junit.After; @@ -254,4 +255,38 @@ public void testLevenshteinDistance() { ERXStringUtilities.levenshteinDistance(l.s1, l.s2)); } } + + @Test + public void testSafeIdentifierName() { + String safeJavaIdentifierStart = "IamSafe"; + String safeJavaIdentifierStartWithUnsafeChars = "Iam Nearly+Safe"; + String unsafeJavaIdentifierStart = "0safe"; + String nullIdentifierStart = null; + String emptyIdentifierStart = ""; + String prefix = "prefix"; + char replacement = '_'; + + String resultWithSafeStart = ERXStringUtilities.safeIdentifierName(safeJavaIdentifierStart, prefix, replacement); + String resultWithSafeStartUnsafeContent = ERXStringUtilities.safeIdentifierName(safeJavaIdentifierStartWithUnsafeChars, prefix, replacement); + String resultWithUnsafeStart = ERXStringUtilities.safeIdentifierName(unsafeJavaIdentifierStart, prefix, replacement); + String resultWithNullStart = ERXStringUtilities.safeIdentifierName(nullIdentifierStart, prefix, replacement); + String resultWithEmptyStart = ERXStringUtilities.safeIdentifierName(emptyIdentifierStart, prefix, replacement); + + Assert.assertEquals(safeJavaIdentifierStart, resultWithSafeStart); + + Assert.assertNotSame(safeJavaIdentifierStartWithUnsafeChars, resultWithSafeStartUnsafeContent); + Assert.assertEquals("Expected 2 replacements for unsafe characters", 2, resultWithSafeStartUnsafeContent.replaceAll("[^_]", "").length()); + Assert.assertFalse("Did not expect prefix as identifier starts with safe character.", resultWithSafeStartUnsafeContent.contains(prefix)); + + Assert.assertNotSame(unsafeJavaIdentifierStart, resultWithUnsafeStart); + Assert.assertFalse("Expected no replacement for unsafe characters", resultWithUnsafeStart.contains("_")); + Assert.assertTrue("Did expect prefix as identifier starts with unsafe character.", resultWithUnsafeStart.contains(prefix)); + + Assert.assertNotSame(nullIdentifierStart, resultWithNullStart); + Assert.assertTrue("Did expect 'null' as identifier was null.", resultWithNullStart.contains("null")); + Assert.assertTrue("Did expect prefix as identifier was null.", resultWithNullStart.contains(prefix)); + + Assert.assertNotSame(emptyIdentifierStart, resultWithEmptyStart); + Assert.assertEquals(prefix, resultWithEmptyStart); + } }