From 2dc420ccf0d0ed17636b3f16cdcaac700bade394 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Mon, 9 Apr 2012 19:18:46 -0400 Subject: [PATCH] If include statements from a cycle, throw a nicer error --- NEWS.md | 2 ++ .../com/typesafe/config/impl/Parseable.java | 34 +++++++++++++++++-- config/src/test/resources/cycle.conf | 1 + .../typesafe/config/impl/PublicApiTest.scala | 9 +++++ 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 config/src/test/resources/cycle.conf diff --git a/NEWS.md b/NEWS.md index ad00b495..1d12a8ce 100644 --- a/NEWS.md +++ b/NEWS.md @@ -30,6 +30,8 @@ implement ConfigIncluderFile, ConfigIncluderURL, and ConfigIncluderClasspath. You should also use ConfigIncludeContext.parseOptions() if appropriate. + - cycles in include statements (self-includes) are now detected + and result in a nicer error instead of stack overflow - the serialization format has changed for a Config that has not had resolve() called on it. The library can still deserialize the old format, but old versions of the library will not be diff --git a/config/src/main/java/com/typesafe/config/impl/Parseable.java b/config/src/main/java/com/typesafe/config/impl/Parseable.java index 09536fa1..95bf8b54 100644 --- a/config/src/main/java/com/typesafe/config/impl/Parseable.java +++ b/config/src/main/java/com/typesafe/config/impl/Parseable.java @@ -20,6 +20,7 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.Enumeration; import java.util.Iterator; +import java.util.LinkedList; import java.util.Properties; import com.typesafe.config.ConfigException; @@ -43,8 +44,16 @@ public abstract class Parseable implements ConfigParseable { private ConfigParseOptions initialOptions; private ConfigOrigin initialOrigin; - protected Parseable() { + private static final ThreadLocal> parseStack = new ThreadLocal>() { + @Override + protected LinkedList initialValue() { + return new LinkedList(); + } + }; + private static final int MAX_INCLUDE_DEPTH = 50; + + protected Parseable() { } private ConfigParseOptions fixupOptions(ConfigParseOptions baseOptions) { @@ -122,7 +131,23 @@ public abstract class Parseable implements ConfigParseable { @Override public ConfigObject parse(ConfigParseOptions baseOptions) { - return forceParsedToObject(parseValue(baseOptions)); + + LinkedList stack = parseStack.get(); + if (stack.size() >= MAX_INCLUDE_DEPTH) { + throw new ConfigException.Parse(initialOrigin, "include statements nested more than " + + MAX_INCLUDE_DEPTH + + " times, you probably have a cycle in your includes. Trace: " + stack); + } + + stack.addFirst(this); + try { + return forceParsedToObject(parseValue(baseOptions)); + } finally { + stack.removeFirst(); + if (stack.isEmpty()) { + parseStack.remove(); + } + } } final AbstractConfigValue parseValue(ConfigParseOptions baseOptions) { @@ -350,6 +375,11 @@ public abstract class Parseable implements ConfigParseable { protected ConfigOrigin createOrigin() { return SimpleConfigOrigin.newSimple("String"); } + + @Override + public String toString() { + return getClass().getSimpleName() + "(" + input + ")"; + } } public static Parseable newString(String input, ConfigParseOptions options) { diff --git a/config/src/test/resources/cycle.conf b/config/src/test/resources/cycle.conf new file mode 100644 index 00000000..a0772910 --- /dev/null +++ b/config/src/test/resources/cycle.conf @@ -0,0 +1 @@ +include "cycle.conf" diff --git a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala index 24aaa30b..205c5a07 100644 --- a/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala @@ -817,4 +817,13 @@ class PublicApiTest extends TestUtils { assertTrue("cache was dropped when switching loaders", load3 ne load7) assertEquals(load3, load7) } + + @Test + def detectIncludeCycle() { + val e = intercept[ConfigException.Parse] { + ConfigFactory.load("cycle") + } + + assertTrue("wrong exception: " + e.getMessage, e.getMessage.contains("include statements nested")) + } }