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 <dun...@wortharead.com> Authored: Wed Dec 14 06:25:07 2016 +0000 Committer: duncan <dun...@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); + } }
