Skip to content

Commit

Permalink
Fix String compressible API loop entry conditions
Browse files Browse the repository at this point in the history 
The loop entry conditions should be traversing the input array starting
from the given offset for the given length number of characters.
Previously passing a starting offset which is greater than the length
would result in the API never entering the loop and always returning
true, even though the input may have contained non-compressible
characters.

We also take this opportunity to rename `compressible` to a better name
which represents what the API is doing, i.e. `canEncodeAsLatin1`.

Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>
  • Loading branch information
@fjeremic
fjeremic committed Dec 11, 2019
1 parent 63f4fcd commit b5085c0
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 28 deletions.
68 changes: 51 additions & 17 deletions jcl/src/java.base/share/classes/java/lang/String.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,25 @@ static void initCompressionFlag() {
}
}

static boolean compressible(char[] c, int start, int length) {
for (int i = start; i < length; ++i) {
/**
* Determines whether the input character array can be encoded as a compact
* Latin1 string.
*
* <p>This API implicitly assumes the following:
* <blockquote><pre>
* - {@code length >= 0}
* - {@code start >= 0}
* - {@code start + length <= data.length}
* <blockquote><pre>
*
* @param c the array of characters to check
* @param start the starting offset in the character array
* @param length the number of characters to check starting at {@code start}
* @return {@code true} if the input character array can be encoded
* using the Latin1 encoding; {@code false} otherwise
*/
static boolean canEncodeAsLatin1(char[] c, int start, int length) {
for (int i = start; i < start + length; ++i) {
if (c[i] > 255) {
return false;
}
Expand Down Expand Up @@ -703,7 +720,7 @@ public String(char[] data) {
* a non-null array of characters
*/
String(char[] data, boolean ignore) {
if (enableCompression && compressible(data, 0, data.length)) {
if (enableCompression && canEncodeAsLatin1(data, 0, data.length)) {
value = new byte[data.length];
coder = LATIN1;

Expand Down Expand Up @@ -738,7 +755,7 @@ public String(char[] data) {
*/
public String(char[] data, int start, int length) {
if (start >= 0 && 0 <= length && length <= data.length - start) {
if (enableCompression && compressible(data, start, length)) {
if (enableCompression && canEncodeAsLatin1(data, start, length)) {
value = new byte[length];
coder = LATIN1;

Expand Down Expand Up @@ -4060,8 +4077,25 @@ static void initCompressionFlag() {
}
}

static boolean compressible(char[] c, int start, int length) {
for (int i = start; i < length; ++i) {
/**
* Determines whether the input character array can be encoded as a compact
* Latin1 string.
*
* <p>This API implicitly assumes the following:
* <blockquote><pre>
* - {@code length >= 0}
* - {@code start >= 0}
* - {@code start + length <= data.length}
* <blockquote><pre>
*
* @param c the array of characters to check
* @param start the starting offset in the character array
* @param length the number of characters to check starting at {@code start}
* @return {@code true} if the input character array can be encoded
* using the Latin1 encoding; {@code false} otherwise
*/
static boolean canEncodeAsLatin1(char[] c, int start, int length) {
for (int i = start; i < start + length; ++i) {
if (c[i] > 255) {
return false;
}
Expand Down Expand Up @@ -4256,7 +4290,7 @@ public String(byte[] data, int start, int length) {
char[] buffer = StringCoding.decode(data, start, length);

if (enableCompression) {
if (compressible(buffer, 0, buffer.length)) {
if (canEncodeAsLatin1(buffer, 0, buffer.length)) {
value = new char[(buffer.length + 1) / 2];
count = buffer.length;

Expand Down Expand Up @@ -4376,7 +4410,7 @@ public String(byte[] data, int start, int length, final String encoding) throws
char[] buffer = StringCoding.decode(encoding, data, start, length);

if (enableCompression) {
if (compressible(buffer, 0, buffer.length)) {
if (canEncodeAsLatin1(buffer, 0, buffer.length)) {
value = new char[(buffer.length + 1) / 2];
count = buffer.length;

Expand Down Expand Up @@ -4487,7 +4521,7 @@ public String(char[] data) {
*/
String(char[] data, boolean ignore) {
if (enableCompression) {
if (compressible(data, 0, data.length)) {
if (canEncodeAsLatin1(data, 0, data.length)) {
value = new char[(data.length + 1) / 2];
count = data.length;

Expand Down Expand Up @@ -4527,7 +4561,7 @@ public String(char[] data) {
public String(char[] data, int start, int length) {
if (start >= 0 && 0 <= length && length <= data.length - start) {
if (enableCompression) {
if (compressible(data, start, length)) {
if (canEncodeAsLatin1(data, start, length)) {
value = new char[(length + 1) / 2];
count = length;

Expand Down Expand Up @@ -7613,28 +7647,28 @@ public String(int[] data, int start, int length) {
int size = 0;

// Optimistically assume we can compress data[]
boolean compressible = enableCompression;
boolean canEncodeAsLatin1 = enableCompression;

for (int i = start; i < start + length; ++i) {
int codePoint = data[i];

if (codePoint < Character.MIN_CODE_POINT) {
throw new IllegalArgumentException();
} else if (codePoint < Character.MIN_SUPPLEMENTARY_CODE_POINT) {
if (compressible && codePoint > 255) {
compressible = false;
if (canEncodeAsLatin1 && codePoint > 255) {
canEncodeAsLatin1 = false;
}

++size;
} else if (codePoint <= Character.MAX_CODE_POINT) {
if (compressible) {
if (canEncodeAsLatin1) {
codePoint -= Character.MIN_SUPPLEMENTARY_CODE_POINT;

int codePoint1 = Character.MIN_HIGH_SURROGATE + (codePoint >> 10);
int codePoint2 = Character.MIN_LOW_SURROGATE + (codePoint & 0x3FF);

if (codePoint1 > 255 || codePoint2 > 255) {
compressible = false;
canEncodeAsLatin1 = false;
}
}

Expand All @@ -7644,7 +7678,7 @@ public String(int[] data, int start, int length) {
}
}

if (compressible) {
if (canEncodeAsLatin1) {
value = new char[(size + 1) / 2];
count = size;

Expand Down Expand Up @@ -8140,7 +8174,7 @@ public String(byte[] data, int start, int length, Charset charset) {
char[] chars = StringCoding.decode(charset, data, start, length);

if (enableCompression) {
if (compressible(chars, 0, chars.length)) {
if (canEncodeAsLatin1(chars, 0, chars.length)) {
value = new char[(chars.length + 1) / 2];
count = chars.length;

Expand Down
10 changes: 5 additions & 5 deletions jcl/src/java.base/share/classes/java/lang/StringBuffer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public synchronized StringBuffer append (char[] chars) {

if (String.enableCompression) {
// Check if the StringBuffer is compressed
if (count >= 0 && String.compressible(chars, 0, chars.length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, 0, chars.length)) {
if (newLength > currentCapacity) {
ensureCapacityImpl(newLength);
}
Expand Down Expand Up @@ -256,7 +256,7 @@ public synchronized StringBuffer append (char chars[], int start, int length) {

if (String.enableCompression) {
// Check if the StringBuffer is compressed
if (count >= 0 && String.compressible(chars, start, length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, start, length)) {
if (newLength > currentCapacity) {
ensureCapacityImpl(newLength);
}
Expand Down Expand Up @@ -840,7 +840,7 @@ public synchronized StringBuffer insert(int index, char[] chars) {

if (String.enableCompression) {
// Check if the StringBuffer is compressed
if (count >= 0 && String.compressible(chars, 0, chars.length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, 0, chars.length)) {
String.compress(chars, 0, value, index, chars.length);

count = currentLength + chars.length;
Expand Down Expand Up @@ -894,7 +894,7 @@ public synchronized StringBuffer insert(int index, char[] chars, int start, int

if (String.enableCompression) {
// Check if the StringBuffer is compressed
if (count >= 0 && String.compressible(chars, start, length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, start, length)) {
String.compress(chars, start, value, index, length);

count = currentLength + length;
Expand Down Expand Up @@ -1788,7 +1788,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo
}

if (String.enableCompression) {
if (String.compressible(streamValue, 0, streamValue.length)) {
if (String.canEncodeAsLatin1(streamValue, 0, streamValue.length)) {
if (streamValue.length == Integer.MAX_VALUE) {
value = new char[(streamValue.length / 2) + 1];
} else {
Expand Down
12 changes: 6 additions & 6 deletions jcl/src/java.base/share/classes/java/lang/StringBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
package java.lang;

/*******************************************************************************
* Copyright (c) 2005, 2018 IBM Corp. and others
* Copyright (c) 2005, 2019 IBM Corp. and others
*
* This program and the accompanying materials are made available under
* the terms of the Eclipse Public License 2.0 which accompanies this
Expand Down Expand Up @@ -198,7 +198,7 @@ public StringBuilder append (char[] chars) {

if (String.enableCompression) {
// Check if the StringBuilder is compressed
if (count >= 0 && String.compressible(chars, 0, chars.length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, 0, chars.length)) {
if (newLength > currentCapacity) {
ensureCapacityImpl(newLength);
}
Expand Down Expand Up @@ -255,7 +255,7 @@ public StringBuilder append (char chars[], int start, int length) {

if (String.enableCompression) {
// Check if the StringBuilder is compressed
if (count >= 0 && String.compressible(chars, start, length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, start, length)) {
if (newLength > currentCapacity) {
ensureCapacityImpl(newLength);
}
Expand Down Expand Up @@ -839,7 +839,7 @@ public StringBuilder insert(int index, char[] chars) {

if (String.enableCompression) {
// Check if the StringBuilder is compressed
if (count >= 0 && String.compressible(chars, 0, chars.length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, 0, chars.length)) {
String.compress(chars, 0, value, index, chars.length);

count = currentLength + chars.length;
Expand Down Expand Up @@ -893,7 +893,7 @@ public StringBuilder insert(int index, char[] chars, int start, int length) {

if (String.enableCompression) {
// Check if the StringBuilder is compressed
if (count >= 0 && String.compressible(chars, start, length)) {
if (count >= 0 && String.canEncodeAsLatin1(chars, start, length)) {
String.compress(chars, start, value, index, length);

count = currentLength + length;
Expand Down Expand Up @@ -1783,7 +1783,7 @@ private void readObject(ObjectInputStream stream) throws IOException, ClassNotFo
}

if (String.enableCompression) {
if (String.compressible(streamValue, 0, streamValue.length)) {
if (String.canEncodeAsLatin1(streamValue, 0, streamValue.length)) {
if (streamValue.length == Integer.MAX_VALUE) {
value = new char[(streamValue.length / 2) + 1];
} else {
Expand Down

0 comments on commit b5085c0

Please sign in to comment.