commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From djo...@apache.org
Subject [1/2] [lang] Fix for LANG-1286: RandomStringUtils random method can overflow...
Date Wed, 14 Dec 2016 06:28:01 GMT
Repository: commons-lang
Updated Branches:
  refs/heads/master 35c27d025 -> 7f7fa03ea


Fix for LANG-1286: RandomStringUtils random method can overflow...

Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/f643b4fa
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/f643b4fa
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/f643b4fa

Branch: refs/heads/master
Commit: f643b4fa939e89348618ddffae20a804f4461363
Parents: f13d18c
Author: duncan <duncan@wortharead.com>
Authored: Wed Dec 14 06:25:07 2016 +0000
Committer: duncan <duncan@wortharead.com>
Committed: Wed Dec 14 06:27:04 2016 +0000

----------------------------------------------------------------------
 src/changes/changes.xml                         |  1 +
 .../apache/commons/lang3/RandomStringUtils.java | 65 ++++++++++----------
 .../commons/lang3/RandomStringUtilsTest.java    | 27 ++++++++
 3 files changed, 60 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-lang/blob/f643b4fa/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c4603fa..0d91422 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -46,6 +46,7 @@ The <action> type attribute can be add,update,fix,remove.
   <body>
 
   <release version="3.6" date="2016-MM-DD" description="TBD">
+    <action issue="LANG-1286" type="fix" dev="djones">RandomStringUtils random method
can overflow and return characters outside of specified range</action>
     <action issue="LANG-660" type="add" dev="djones">Add methods to insert arrays into
arrays at an index</action>
     <action issue="LANG-1292" type="fix" dev="djones">WordUtils.wrap throws StringIndexOutOfBoundsException</action>
     <action issue="LANG-1287" type="fix" dev="pschumacher" due-to="Ivan Morozov">RandomStringUtils#random
can enter infinite loop if end parameter is to small</action>

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/f643b4fa/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/lang3/RandomStringUtils.java b/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
index b76e269..bbb5e78 100644
--- a/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
+++ b/src/main/java/org/apache/commons/lang3/RandomStringUtils.java
@@ -315,7 +315,7 @@ public class RandomStringUtils {
      * to {@code ' '} and {@code 'z'}, the ASCII printable
      * characters, will be used, unless letters and numbers are both
      * {@code false}, in which case, start and end are set to
-     * {@code 0} and {@code Integer.MAX_VALUE}.
+     * {@code 0} and {@link Character#MAX_CODE_POINT}.
      *
      * <p>If set is not {@code null}, characters between start and
      * end are chosen.</p>
@@ -356,7 +356,7 @@ public class RandomStringUtils {
                 end = chars.length;
             } else {
                 if (!letters && !numbers) {
-                    end = Integer.MAX_VALUE;
+                    end = Character.MAX_CODE_POINT;
                 } else {
                     end = 'z' + 1;
                     start = ' ';                
@@ -379,50 +379,49 @@ public class RandomStringUtils {
             }
         }
 
-        final char[] buffer = new char[count];
+        StringBuffer buffer = new StringBuffer(count);
         final int gap = end - start;
 
         while (count-- != 0) {
-            char ch;
+            int codePoint;
             if (chars == null) {
-                ch = (char) (random.nextInt(gap) + start);
+                codePoint = random.nextInt(gap) + start;
+                
+                switch (Character.getType(codePoint)) {
+                case Character.UNASSIGNED:
+                case Character.PRIVATE_USE:
+                case Character.SURROGATE:
+                    count++;
+                    continue;
+                }
+                
             } else {
-                ch = chars[random.nextInt(gap) + start];
+                codePoint = chars[random.nextInt(gap) + start];
             }
-            if (letters && Character.isLetter(ch)
-                    || numbers && Character.isDigit(ch)
-                    || !letters && !numbers) {
-                if(ch >= 56320 && ch <= 57343) {
-                    if(count == 0) {
-                        count++;
-                    } else {
-                        // low surrogate, insert high surrogate after putting it in
-                        buffer[count] = ch;
-                        count--;
-                        buffer[count] = (char) (55296 + random.nextInt(128));
-                    }
-                } else if(ch >= 55296 && ch <= 56191) {
-                    if(count == 0) {
-                        count++;
-                    } else {
-                        // high surrogate, insert low surrogate before putting it in
-                        buffer[count] = (char) (56320 + random.nextInt(128));
-                        count--;
-                        buffer[count] = ch;
-                    }
-                } else if(ch >= 56192 && ch <= 56319) {
-                    // private high surrogate, no effing clue, so skip it
-                    count++;
-                } else {
-                    buffer[count] = ch;
+            
+            final int numberOfChars = Character.charCount(codePoint);
+            if (count == 0 && numberOfChars > 1) {
+                count++;
+                continue;
+            }
+            
+            if (letters && Character.isLetter(codePoint)
+                    || numbers && Character.isDigit(codePoint)
+                    || !letters && !numbers) {               
+                buffer.appendCodePoint(codePoint);
+                
+                if (numberOfChars == 2) {
+                    count--;
                 }
+                
             } else {
                 count++;
             }
         }
-        return new String(buffer);
+        return buffer.toString();
     }
 
+
     /**
      * <p>Creates a random string whose length is the number of characters
      * specified.</p>

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/f643b4fa/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java b/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
index 4aff749..961eb2d 100644
--- a/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java
@@ -533,5 +533,32 @@ public class RandomStringUtilsTest {
         // just to be complete
         assertEquals(orig, copy);
     }
+    
+    
+    /**
+     * Test for LANG-1286. Creates situation where old code would
+     * overflow a char and result in a code point outside the specified
+     * range.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testCharOverflow() throws Exception {
+        int start = Character.MAX_VALUE;
+        int end = Integer.MAX_VALUE;
+        
+        @SuppressWarnings("serial")
+        Random fixedRandom = new Random() {
+            @Override
+            public int nextInt(int n) {
+                // Prevents selection of 'start' as the character
+                return super.nextInt(n - 1) + 1;
+            }
+        };
+        
+        String result = RandomStringUtils.random(2, start, end, false, false, null, fixedRandom);
+        int c = result.codePointAt(0);
+        assertTrue(String.format("Character '%d' not in range [%d,%d).", c, start, end),
c >= start && c < end);
+    }
 }
 


Mime
View raw message