From cd815847e408c01bf211ca6a7f1b84c08fb979d2 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Sun, 21 Jan 2024 18:06:28 +0100 Subject: [PATCH 1/4] fix parseManyAliasesForCollections * Decrease minDuration - 700ms for slower cores seems too slow. * replace manual time measurement with Java 8 time utils * add a JUnit timeout assertion --- .../usecases/references/ReferencesTest.java | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java index ef130c04e..cfdc5038e 100644 --- a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java +++ b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java @@ -13,11 +13,6 @@ */ package org.snakeyaml.engine.usecases.references; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; - -import java.util.LinkedHashMap; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -26,6 +21,12 @@ import org.snakeyaml.engine.v2.api.Load; import org.snakeyaml.engine.v2.api.LoadSettings; +import java.time.Duration; +import java.time.Instant; +import java.util.LinkedHashMap; + +import static org.junit.jupiter.api.Assertions.*; + @Tag("fast") public class ReferencesTest { @@ -85,7 +86,7 @@ public void referencesWithRecursiveKeysNotAllowedByDefault() { fail(); } catch (Exception e) { assertEquals("Recursive key for mapping is detected but it is not configured to be allowed.", - e.getMessage()); + e.getMessage()); } long time2 = System.currentTimeMillis(); float duration = (time2 - time1) / 1000; @@ -95,22 +96,23 @@ public void referencesWithRecursiveKeysNotAllowedByDefault() { @Test @DisplayName("Parsing with aliases may take a lot of time, CPU and memory") public void parseManyAliasesForCollections() { - String output = createDump(25); - // Load - long time1 = System.currentTimeMillis(); - LoadSettings settings = + final var minDuration = Duration.ofMillis(400); + final var maxDuration = Duration.ofSeconds(9); + assertTimeout(maxDuration, () -> { + String output = createDump(25); + // Load + final var start = Instant.now(); + LoadSettings settings = LoadSettings.builder().setAllowRecursiveKeys(true).setMaxAliasesForCollections(50).build(); - Load load = new Load(settings); - load.loadFromString(output); - long time2 = System.currentTimeMillis(); - double duration = (time2 - time1) / 1000.0; - int cores = Runtime.getRuntime().availableProcessors(); - double minDuration = 0.7; - if (cores > 4) { - minDuration = 0.4; - } - assertTrue(duration > minDuration, "It should take time. Time was " + duration + " seconds."); - assertTrue(duration < 9.0, "Time was " + duration + " seconds."); + Load load = new Load(settings); + load.loadFromString(output); + final var finish = Instant.now(); + final var timeElapsed = Duration.between(start, finish); + assertTrue( + timeElapsed.compareTo(minDuration) > 0, /* timeElapsed > minDuration */ + "Expected elapsed time (" + (timeElapsed.toMillis() / 1000f) + ") seconds > minDuration (" + (minDuration.toMillis() / 1000f) + " seconds)" + ); + }); } @Test @@ -121,14 +123,14 @@ public void referencesWithRestrictedAliases() { // Load long time1 = System.currentTimeMillis(); LoadSettings settings = - LoadSettings.builder().setAllowRecursiveKeys(true).setMaxAliasesForCollections(40).build(); + LoadSettings.builder().setAllowRecursiveKeys(true).setMaxAliasesForCollections(40).build(); Load load = new Load(settings); try { load.loadFromString(bigYAML); fail(); } catch (Exception e) { assertEquals("Number of aliases for non-scalar nodes exceeds the specified max=40", - e.getMessage()); + e.getMessage()); } long time2 = System.currentTimeMillis(); float duration = (time2 - time1) / 1000; From 2d534ccddb84c553f4d347d1ff6e970532fb4e42 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Sun, 21 Jan 2024 18:11:16 +0100 Subject: [PATCH 2/4] * increase max duration --- .../snakeyaml/engine/usecases/references/ReferencesTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java index cfdc5038e..9397da92b 100644 --- a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java +++ b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java @@ -97,7 +97,7 @@ public void referencesWithRecursiveKeysNotAllowedByDefault() { @DisplayName("Parsing with aliases may take a lot of time, CPU and memory") public void parseManyAliasesForCollections() { final var minDuration = Duration.ofMillis(400); - final var maxDuration = Duration.ofSeconds(9); + final var maxDuration = Duration.ofSeconds(15); assertTimeout(maxDuration, () -> { String output = createDump(25); // Load From 8e3b7d15f18bb1574de5e8148d881b50f65ab5b3 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Sun, 21 Jan 2024 18:33:24 +0100 Subject: [PATCH 3/4] * tidy up code warnings --- .../usecases/references/ReferencesTest.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java index 9397da92b..adba3f92e 100644 --- a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java +++ b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java @@ -37,18 +37,18 @@ public class ReferencesTest { * @return YAML to parse */ private String createDump(int size) { - LinkedHashMap root = new LinkedHashMap(); - LinkedHashMap s1, s2, t1, t2; + LinkedHashMap root = new LinkedHashMap<>(); + LinkedHashMap s1, s2, t1, t2; s1 = root; - s2 = new LinkedHashMap(); + s2 = new LinkedHashMap<>(); /* * the time to parse grows very quickly SIZE -> time to parse in seconds 25 -> 1 26 -> 2 27 -> 3 * 28 -> 8 29 -> 13 30 -> 28 31 -> 52 32 -> 113 33 -> 245 34 -> 500 */ for (int i = 0; i < size; i++) { - t1 = new LinkedHashMap(); - t2 = new LinkedHashMap(); + t1 = new LinkedHashMap<>(); + t2 = new LinkedHashMap<>(); t1.put("foo", "1"); t2.put("bar", "2"); @@ -63,7 +63,8 @@ private String createDump(int size) { // this is VERY BAD code // the map has itself as a key (no idea why it may be used except of a DoS attack) - LinkedHashMap f = new LinkedHashMap(); + LinkedHashMap f = new LinkedHashMap<>(); + //noinspection CollectionAddedToSelf f.put(f, "a"); f.put("g", root); @@ -89,7 +90,7 @@ public void referencesWithRecursiveKeysNotAllowedByDefault() { e.getMessage()); } long time2 = System.currentTimeMillis(); - float duration = (time2 - time1) / 1000; + float duration = (time2 - time1) / 1000f; assertTrue(duration < 1.0, "It should fail quickly. Time was " + duration + " seconds."); } @@ -133,7 +134,7 @@ public void referencesWithRestrictedAliases() { e.getMessage()); } long time2 = System.currentTimeMillis(); - float duration = (time2 - time1) / 1000; + float duration = (time2 - time1) / 1000f; assertTrue(duration < 1.0, "It should fail quickly. Time was " + duration + " seconds."); } } From 5b7a8ccb461ab8823d3d41bc95292b565c544874 Mon Sep 17 00:00:00 2001 From: Adam <897017+aSemy@users.noreply.github.com> Date: Sun, 21 Jan 2024 18:34:07 +0100 Subject: [PATCH 4/4] return elapsed time from the assertTimeout lambda (assertions seem to be swallowed when they're inside the timed lambda?) --- .../engine/usecases/references/ReferencesTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java index adba3f92e..0448ee3e1 100644 --- a/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java +++ b/src/jvmTest/java/org/snakeyaml/engine/usecases/references/ReferencesTest.java @@ -99,7 +99,7 @@ public void referencesWithRecursiveKeysNotAllowedByDefault() { public void parseManyAliasesForCollections() { final var minDuration = Duration.ofMillis(400); final var maxDuration = Duration.ofSeconds(15); - assertTimeout(maxDuration, () -> { + final var timeElapsed = assertTimeout(maxDuration, () -> { String output = createDump(25); // Load final var start = Instant.now(); @@ -108,12 +108,12 @@ public void parseManyAliasesForCollections() { Load load = new Load(settings); load.loadFromString(output); final var finish = Instant.now(); - final var timeElapsed = Duration.between(start, finish); - assertTrue( - timeElapsed.compareTo(minDuration) > 0, /* timeElapsed > minDuration */ - "Expected elapsed time (" + (timeElapsed.toMillis() / 1000f) + ") seconds > minDuration (" + (minDuration.toMillis() / 1000f) + " seconds)" - ); + return Duration.between(start, finish); }); + assertTrue( + timeElapsed.compareTo(minDuration) > 0, /* timeElapsed > minDuration */ + "Expected elapsed time (" + (timeElapsed.toMillis() / 1000f) + ") seconds > minDuration (" + (minDuration.toMillis() / 1000f) + " seconds)" + ); } @Test