avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nkol...@apache.org
Subject avro git commit: AVRO-2122: Cannot validate schemas with recursive definitions
Date Tue, 23 Jan 2018 10:50:49 GMT
Repository: avro
Updated Branches:
  refs/heads/branch-1.8 a731fab50 -> 49471412e


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 #276

Signed-off-by: Nandor Kollar <nkollar@apache.org>

(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/49471412
Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/49471412
Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/49471412

Branch: refs/heads/branch-1.8
Commit: 49471412e5a10ff7b4f2806a1af03372d12b7945
Parents: a731fab
Author: Bart <brtm@users.noreply.github.com>
Authored: Wed Jan 3 15:03:57 2018 +0100
Committer: Nandor Kollar <nkollar@apache.org>
Committed: Tue Jan 23 11:32:38 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/49471412/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index b65e76c..7156f73 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -37,6 +37,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.2 (10 April 2017)
 
   INCOMPATIBLE CHANGES

http://git-wip-us.apache.org/repos/asf/avro/blob/49471412/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 f33c292..106352a 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/49471412/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 838dc91..8456c1d 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
@@ -203,5 +203,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));
+  }
 }


Mime
View raw message