Skip to content

Commit

Permalink
Fix ExpressionInfoValueRenderer.renderAsFailedStringComparison overflow
Browse files Browse the repository at this point in the history
The length check in renderAsFailedStringComparison did overflow, so we
need to cast it to long first. Also add some more info when the
OOM protection kicks in. Furthermore, decrease the the memory limit from
1mb to 50kb. And add some specs for tryReduceStringSizes.
  • Loading branch information
leonard84 committed Apr 19, 2017
1 parent b32bd02 commit e62a8de
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@
import org.spockframework.runtime.condition.EditDistance;
import org.spockframework.runtime.condition.EditPathRenderer;
import org.spockframework.runtime.model.ExpressionInfo;
import org.spockframework.runtime.GroovyRuntimeUtil;
import org.spockframework.util.ObjectUtil;
import org.spockframework.util.Nullable;
import org.spockframework.util.ObjectUtil;

public class ExpressionInfoValueRenderer {
public static final long MAX_EDIT_DISTANCE_MEMORY = 1024*1024;
public static final long MAX_EDIT_DISTANCE_MEMORY = 50 * 1024;
private final ExpressionInfo expr;

private ExpressionInfoValueRenderer(ExpressionInfo expr) {
Expand Down Expand Up @@ -88,13 +87,13 @@ private String javaLangObjectToString(Object value) {
private String doRenderValue(ExpressionInfo expr) {
String result = renderAsFailedStringComparison(expr);
if (result != null) return result;

result = renderAsFailedEqualityComparison(expr);
if (result != null) return result;

return GroovyRuntimeUtil.toString(expr.getValue());
}

private String renderAsFailedStringComparison(ExpressionInfo expr) {
if (!(Boolean.FALSE.equals(expr.getValue()))) return null;
if (!expr.isEqualityComparison(String.class, GString.class)) return null;
Expand All @@ -103,7 +102,7 @@ private String renderAsFailedStringComparison(ExpressionInfo expr) {
String str1 = expr.getChildren().get(0).getValue().toString();
String str2 = expr.getChildren().get(1).getValue().toString();

if (str1.length() * str2.length() > MAX_EDIT_DISTANCE_MEMORY) {
if (((long)str1.length()) * str2.length() > MAX_EDIT_DISTANCE_MEMORY) {
return tryReduceStringSizes(str1, str2);
} else {
return createAndRenderEditDistance(str1, str2);
Expand Down Expand Up @@ -137,7 +136,7 @@ private String tryReduceStringSizes(String str1, String str2) {
end1 = Math.min(str1.length(), end1 + 10);
end2 = Math.min(str2.length(), end2 + 10);
}
return createAndRenderEditDistance(str1.substring(commonStart, end1), str2.substring(commonStart, end2));
return createAndRenderEditDistance(str1, str2, commonStart, end1, end2);
}
}

Expand All @@ -148,6 +147,16 @@ private String createAndRenderEditDistance(String str1, String str2) {
new EditPathRenderer().render(str1, str2, dist.calculatePath()));
}

private String createAndRenderEditDistance(String str1, String str2, int commonStart, int end1, int end2) {
String sub1 = str1.substring(commonStart, end1);
String sub2 = str2.substring(commonStart, end2);
EditDistance dist = new EditDistance(sub1, sub2);
return String.format("false\n%d difference%s (%d%% similarity) (comparing subset start: %d, end1: %d, end2: %d)\n%s",
dist.getDistance(), dist.getDistance() == 1 ? "" : "s", dist.getSimilarityInPercent(),
commonStart, end1, end2,
new EditPathRenderer().render(sub1, sub2, dist.calculatePath()));
}

private String renderAsFailedEqualityComparison(ExpressionInfo expr) {
if (!(Boolean.FALSE.equals(expr.getValue()))) return null;
if (!expr.isEqualityComparison()) return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@

package org.spockframework.smoke.condition

import org.spockframework.runtime.ConditionNotSatisfiedError
import org.spockframework.runtime.Condition

import org.spockframework.runtime.ConditionNotSatisfiedError
import spock.lang.Specification

/**
Expand All @@ -41,4 +40,16 @@ abstract class ConditionRenderingSpec extends Specification {

assert false, "condition should have failed but didn't"
}
}

void renderedConditionContains(Closure condition, String... contents) {
try {
condition()
} catch (ConditionNotSatisfiedError e) {
String rendering = e.condition.rendering.trim()
contents.each { assert rendering.contains(it)}
return
}

assert false, "condition should have failed but didn't"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.spockframework.smoke.condition

import org.spockframework.runtime.ExpressionInfoValueRenderer
import spock.lang.Issue

class StringComparisonRendering extends ConditionRenderingSpec {
Expand Down Expand Up @@ -78,4 +79,96 @@ null == "foo"
assert ("the quick" == "the quick") instanceof String
}
}
@Issue("https://github.com/spockframework/spock/issues/121")
def "large String comparision does not cause OOM-Error, difference at start"() {
StringBuilder sb = largeStringBuilder()
String a = sb.toString()
sb.setCharAt(0, 'b' as char)
String b = sb.toString()

expect:
renderedConditionContains({
assert a == b
},
"1 difference (90% similarity) (comparing subset start: 0, end1: 11, end2: 11)",
"(a)aaaaaaaaaa",
"(b)aaaaaaaaaa"
)
}

@Issue("https://github.com/spockframework/spock/issues/121")
def "large String comparision does not cause OOM-Error, difference in the middle"() {
StringBuilder sb = largeStringBuilder()
String a = sb.toString()
sb.setCharAt(sb.length() / 2 as int, 'b' as char)
String b = sb.toString()

expect:
renderedConditionContains({
assert a == b
},
"1 difference (95% similarity) (comparing subset start: 12789, end1: 12811, end2: 12811)",
"aaaaaaaaaaa(a)aaaaaaaaaa",
"aaaaaaaaaaa(b)aaaaaaaaaa"
)
}

@Issue("https://github.com/spockframework/spock/issues/121")
def "large String comparision does not cause OOM-Error, difference at end"() {
StringBuilder sb = largeStringBuilder()
String a = sb.toString()
sb.setCharAt(sb.length()-1, 'b' as char)
String b = sb.toString()

expect:
renderedConditionContains({
assert a == b
},
"1 difference (91% similarity) (comparing subset start: 25588, end1: 25600, end2: 25600)",
"aaaaaaaaaaa(a)",
"aaaaaaaaaaa(b)")
}



@Issue("https://github.com/spockframework/spock/issues/121")
def "large String comparision does not cause OOM-Error, difference at start and end"() {
StringBuilder sb = largeStringBuilder()
String a = sb.toString()
sb.setCharAt(0, 'b' as char)
sb.setCharAt(sb.length()-1, 'b' as char)
String b = sb.toString()

expect:
renderedConditionContains({
assert a == b
},
"false",
"Strings too large to calculate edit distance.")
}


@Issue("https://github.com/spockframework/spock/issues/121")
def "large String comparision does not cause OOM-Error, complete difference"() {
String a = largeStringBuilder()
String b = largeStringBuilder("bbbbbbbbbbbbbbbb")


expect:
renderedConditionContains({
assert a == b
},
"false",
"Strings too large to calculate edit distance.")
}

private StringBuilder largeStringBuilder(CharSequence source = "aaaaaaaaaaaaaaaa") {
int length = ExpressionInfoValueRenderer.MAX_EDIT_DISTANCE_MEMORY / 2
def sb = new StringBuilder(length + 10)
int cslength = source.length()
for (int i = 0; i < length; i += cslength) {
sb.append(source, 0, Math.min(length - i, cslength))
}
return sb
}
}

0 comments on commit e62a8de

Please sign in to comment.