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:08:38 GMT
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 <nkollar@apache.org>


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 <brtm@users.noreply.github.com>
Authored: Wed Jan 3 15:03:57 2018 +0100
Committer: Nandor Kollar <nkollar@apache.org>
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));
+  }
 }


Mime
View raw message