Repository: avro Updated Branches: refs/heads/branch-1.7 4f1ccf4d1 -> b2e16953f
AVRO-2122: Cannot validate schemas with recursive definitions Track which symbols have been visited to avoid StackOverflowErrors when validating schemas with recursive definitions Signed-off-by: Nandor Kollar <[email protected]> (cherry picked from commit 7f9cbca12af13d4b8b5709edba2bae4d4a808102) Project: http://git-wip-us.apache.org/repos/asf/avro/repo Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/b2e16953 Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/b2e16953 Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/b2e16953 Branch: refs/heads/branch-1.7 Commit: b2e16953f6711d13a4b1bdd43c169f4067e091a6 Parents: 4f1ccf4 Author: Bart <[email protected]> Authored: Wed Jan 3 15:03:57 2018 +0100 Committer: Nandor Kollar <[email protected]> Committed: Tue Jan 23 13:00:00 2018 +0100 ---------------------------------------------------------------------- CHANGES.txt | 3 +++ .../java/org/apache/avro/io/parsing/Symbol.java | 24 +++++++++++++++----- .../org/apache/avro/TestSchemaValidation.java | 11 +++++++++ 3 files changed, 32 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/avro/blob/b2e16953/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 19531ef..d00de56 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -80,6 +80,9 @@ Trunk (not yet released) AVRO-1407: Java: Fix infinite loop on slow connect in NettyTransceiver. (Gareth Davis via cutting) + AVRO-2122: Cannot validate schemas with recursive definitions + (Bart via Nandor Kollar) + Avro 1.7.7 (23 July 2014) NEW FEATURES http://git-wip-us.apache.org/repos/asf/avro/blob/b2e16953/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 92b2dab..de4808d 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; @@ -355,9 +357,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: @@ -366,16 +378,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: @@ -383,13 +395,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/b2e16953/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 9e25767..12e2c9b 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 @@ -160,5 +160,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)); + } }
