Repository: avro Updated Branches: refs/heads/master 37bd84dc6 -> 7f9cbca12
AVRO-2122: Cannot validate schemas with recursive definitions Track which symbols have been visited to avoid StackOverflowErrors when validating schemas with recursive definitions This closes #281 Signed-off-by: Nandor Kollar <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/avro/repo Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/7f9cbca1 Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/7f9cbca1 Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/7f9cbca1 Branch: refs/heads/master Commit: 7f9cbca12af13d4b8b5709edba2bae4d4a808102 Parents: 37bd84d Author: Bart <[email protected]> Authored: Wed Jan 3 15:03:57 2018 +0100 Committer: Nandor Kollar <[email protected]> Committed: Tue Jan 23 10:48:17 2018 +0100 ---------------------------------------------------------------------- CHANGES.txt | 3 +++ .../java/org/apache/avro/io/parsing/Symbol.java | 24 +++++++++++++++----- .../org/apache/avro/TestSchemaValidation.java | 12 ++++++++++ 3 files changed, 33 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/avro/blob/7f9cbca1/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 3761144..e7d63ee 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -226,6 +226,9 @@ Trunk (not yet released) AVRO-2120: Fix NullPointerException thrown by Schema.Parser#parse("") + AVRO-2122: Cannot validate schemas with recursive definitions + (Bart via Nandor Kollar) + Avro 1.8.1 (14 May 2016) INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/avro/blob/7f9cbca1/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java ---------------------------------------------------------------------- diff --git a/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java b/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java index dac2b26..1879424 100644 --- a/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java +++ b/lang/java/avro/src/main/java/org/apache/avro/io/parsing/Symbol.java @@ -19,10 +19,12 @@ package org.apache.avro.io.parsing; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Set; import org.apache.avro.Schema; @@ -369,9 +371,19 @@ public abstract class Symbol { * for some inputs. */ public static boolean hasErrors(Symbol symbol) { + return hasErrors(symbol, new HashSet<Symbol>()); + } + + private static boolean hasErrors(Symbol symbol, Set<Symbol> visited) { + // avoid infinite recursion + if (visited.contains(symbol)) { + return false; + } + visited.add(symbol); + switch(symbol.kind) { case ALTERNATIVE: - return hasErrors(symbol, ((Alternative) symbol).symbols); + return hasErrors(symbol, ((Alternative) symbol).symbols, visited); case EXPLICIT_ACTION: return false; case IMPLICIT_ACTION: @@ -380,16 +392,16 @@ public abstract class Symbol { } if (symbol instanceof UnionAdjustAction) { - return hasErrors(((UnionAdjustAction) symbol).symToParse); + return hasErrors(((UnionAdjustAction) symbol).symToParse, visited); } return false; case REPEATER: Repeater r = (Repeater) symbol; - return hasErrors(r.end) || hasErrors(symbol, r.production); + return hasErrors(r.end, visited) || hasErrors(symbol, r.production, visited); case ROOT: case SEQUENCE: - return hasErrors(symbol, symbol.production); + return hasErrors(symbol, symbol.production, visited); case TERMINAL: return false; default: @@ -397,13 +409,13 @@ public abstract class Symbol { } } - private static boolean hasErrors(Symbol root, Symbol[] symbols) { + private static boolean hasErrors(Symbol root, Symbol[] symbols, Set<Symbol> visited) { if(null != symbols) { for(Symbol s: symbols) { if (s == root) { continue; } - if (hasErrors(s)) { + if (hasErrors(s, visited)) { return true; } } http://git-wip-us.apache.org/repos/asf/avro/blob/7f9cbca1/lang/java/avro/src/test/java/org/apache/avro/TestSchemaValidation.java ---------------------------------------------------------------------- diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaValidation.java b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaValidation.java index 1673537..fc933c9 100644 --- a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaValidation.java +++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaValidation.java @@ -407,4 +407,16 @@ public class TestSchemaValidation { } Assert.assertTrue(threw); } + public static final org.apache.avro.Schema recursiveSchema = new org.apache.avro.Schema.Parser().parse("{\"type\":\"record\",\"name\":\"Node\",\"namespace\":\"avro\",\"fields\":[{\"name\":\"value\",\"type\":[\"null\",\"Node\"],\"default\":null}]}"); + + /** + * Unit test to verify that recursive schemas can be validated. + * See AVRO-2122. + */ + @Test + public void testRecursiveSchemaValidation() throws SchemaValidationException { + // before AVRO-2122, this would cause a StackOverflowError + final SchemaValidator backwardValidator = builder.canReadStrategy().validateLatest(); + backwardValidator.validate(recursiveSchema, Arrays.asList(recursiveSchema)); + } }
