This is an automated email from the ASF dual-hosted git repository.
rskraba pushed a commit to branch branch-1.11
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/branch-1.11 by this push:
new 64ecf4a AVRO-2867: Fix NullPointerException on record-valued defaults
(#1412)
64ecf4a is described below
commit 64ecf4ab19c57fa3e0d1596368e8f535727c6623
Author: Oscar Westra van Holthe - Kind <[email protected]>
AuthorDate: Wed Dec 22 08:58:43 2021 +0100
AVRO-2867: Fix NullPointerException on record-valued defaults (#1412)
If the IDL file defines the schema of a field after the field, record
valued defaults cause a NullPointerException. This PR fixes that.
The fix addresses two situations:
1. The field schema itself is a forward reference
(tested by fixing the missing default value in `forward_ref.avpr`)
2. The field schema contains a forward reference
(tested by the `echo` message in the updated `simple.avdl`)
---
.../avro/src/main/java/org/apache/avro/Schema.java | 12 +++++
.../avro/compiler/idl/IsResolvedSchemaVisitor.java | 62 ++++++++++++++++++++++
.../apache/avro/compiler/idl/ResolvingVisitor.java | 5 +-
.../apache/avro/compiler/idl/SchemaResolver.java | 14 +++++
.../org/apache/avro/compiler/schema/Schemas.java | 6 +--
.../javacc/org/apache/avro/compiler/idl/idl.jj | 2 +-
lang/java/compiler/src/test/idl/input/simple.avdl | 6 +--
.../compiler/src/test/idl/output/forward_ref.avpr | 2 +-
lang/java/compiler/src/test/idl/output/simple.avpr | 12 ++---
9 files changed, 102 insertions(+), 19 deletions(-)
diff --git a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index 76ee2ee..fec08e1 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -282,6 +282,13 @@ public abstract class Schema extends JsonProperties
implements Serializable {
}
/**
+ * If this is a record, returns whether the fields have been set.
+ */
+ public boolean hasFields() {
+ throw new AvroRuntimeException("Not a record: " + this);
+ }
+
+ /**
* If this is a record, set its fields. The fields can be set only once in a
* schema.
*/
@@ -905,6 +912,11 @@ public abstract class Schema extends JsonProperties
implements Serializable {
}
@Override
+ public boolean hasFields() {
+ return fields != null;
+ }
+
+ @Override
public void setFields(List<Field> fields) {
if (this.fields != null) {
throw new AvroRuntimeException("Fields are already set");
diff --git
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/IsResolvedSchemaVisitor.java
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/IsResolvedSchemaVisitor.java
new file mode 100644
index 0000000..6006ad5
--- /dev/null
+++
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/IsResolvedSchemaVisitor.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.avro.compiler.idl;
+
+import org.apache.avro.Schema;
+import org.apache.avro.compiler.schema.SchemaVisitor;
+import org.apache.avro.compiler.schema.SchemaVisitorAction;
+
+/**
+ * This visitor checks if the current schema is fully resolved.
+ */
+public final class IsResolvedSchemaVisitor implements SchemaVisitor<Boolean> {
+ boolean hasUnresolvedParts;
+
+ IsResolvedSchemaVisitor() {
+ hasUnresolvedParts = false;
+ }
+
+ @Override
+ public SchemaVisitorAction visitTerminal(Schema terminal) {
+ hasUnresolvedParts = SchemaResolver.isUnresolvedSchema(terminal);
+ return hasUnresolvedParts ? SchemaVisitorAction.TERMINATE :
SchemaVisitorAction.CONTINUE;
+ }
+
+ @Override
+ public SchemaVisitorAction visitNonTerminal(Schema nonTerminal) {
+ hasUnresolvedParts = SchemaResolver.isUnresolvedSchema(nonTerminal);
+ if (hasUnresolvedParts) {
+ return SchemaVisitorAction.TERMINATE;
+ }
+ if (nonTerminal.getType() == Schema.Type.RECORD &&
!nonTerminal.hasFields()) {
+ // We're still initializing the type...
+ return SchemaVisitorAction.SKIP_SUBTREE;
+ }
+ return SchemaVisitorAction.CONTINUE;
+ }
+
+ @Override
+ public SchemaVisitorAction afterVisitNonTerminal(Schema nonTerminal) {
+ return SchemaVisitorAction.CONTINUE;
+ }
+
+ @Override
+ public Boolean get() {
+ return !hasUnresolvedParts;
+ }
+}
diff --git
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/ResolvingVisitor.java
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/ResolvingVisitor.java
index c00252e..1c71754 100644
---
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/ResolvingVisitor.java
+++
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/ResolvingVisitor.java
@@ -139,10 +139,7 @@ public final class ResolvingVisitor implements
SchemaVisitor<Schema> {
List<Schema.Field> fields = nt.getFields();
List<Schema.Field> newFields = new ArrayList<>(fields.size());
for (Schema.Field field : fields) {
- Schema.Field newField = new Schema.Field(field.name(),
replace.get(field.schema()), field.doc(),
- field.defaultVal(), field.order());
- copyAllProperties(field, newField);
- newFields.add(newField);
+ newFields.add(new Field(field, replace.get(field.schema())));
}
newSchema.setFields(newFields);
}
diff --git
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
index 2da4944..193f871 100644
---
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
+++
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/SchemaResolver.java
@@ -84,6 +84,20 @@ final class SchemaResolver {
}
/**
+ * Is this a unresolved schema.
+ *
+ * @param schema
+ * @return
+ */
+ static boolean isFullyResolvedSchema(final Schema schema) {
+ if (isUnresolvedSchema(schema)) {
+ return false;
+ } else {
+ return Schemas.visit(schema, new IsResolvedSchemaVisitor());
+ }
+ }
+
+ /**
* Will clone the provided protocol while resolving all unreferenced schemas
*
* @param protocol
diff --git
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/schema/Schemas.java
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/schema/Schemas.java
index 91232f0..b35adbd 100644
---
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/schema/Schemas.java
+++
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/schema/Schemas.java
@@ -21,7 +21,6 @@ import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Deque;
import java.util.IdentityHashMap;
-import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
@@ -141,9 +140,8 @@ public final class Schemas {
visited.put(schema, schema);
break;
case RECORD:
- Iterator<Schema> reverseSchemas =
schema.getFields().stream().map(Field::schema)
-
.collect(Collectors.toCollection(ArrayDeque::new)).descendingIterator();
- terminate = visitNonTerminal(visitor, schema, dq, () ->
reverseSchemas);
+ terminate = visitNonTerminal(visitor, schema, dq, () ->
schema.getFields().stream().map(Field::schema)
+
.collect(Collectors.toCollection(ArrayDeque::new)).descendingIterator());
visited.put(schema, schema);
break;
case UNION:
diff --git
a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
index 667eece..32fc2fa 100644
--- a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
+++ b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
@@ -1348,7 +1348,7 @@ void VariableDeclarator(Schema type, List<Field> fields):
if ("order".equals(key))
order =
Field.Order.valueOf(getTextProp(key,props,token).toUpperCase());
- boolean validate = !SchemaResolver.isUnresolvedSchema(type);
+ boolean validate = SchemaResolver.isFullyResolvedSchema(type);
Field field = Accessor.createField(name, type, DocCommentHelper.getDoc(),
defaultValue, validate, order);
for (String key : props.keySet())
if ("order".equals(key)) { // already handled: ignore
diff --git a/lang/java/compiler/src/test/idl/input/simple.avdl
b/lang/java/compiler/src/test/idl/input/simple.avdl
index c0df9f3..5208c7e 100644
--- a/lang/java/compiler/src/test/idl/input/simple.avdl
+++ b/lang/java/compiler/src/test/idl/input/simple.avdl
@@ -36,9 +36,6 @@ protocol Simple {
C
} = C;
- /** An MD5 hash. */
- fixed MD5(16);
-
/** A TestRecord. */
@my-property({"key":3})
record TestRecord {
@@ -64,6 +61,9 @@ protocol Simple {
union {null, @foo.foo.bar(42) @foo.foo.foo("3foo") string} prop = null;
}
+ /** An MD5 hash. */
+ fixed MD5(16);
+
error TestError {
string message;
}
diff --git a/lang/java/compiler/src/test/idl/output/forward_ref.avpr
b/lang/java/compiler/src/test/idl/output/forward_ref.avpr
index 06531b5..a349206 100644
--- a/lang/java/compiler/src/test/idl/output/forward_ref.avpr
+++ b/lang/java/compiler/src/test/idl/output/forward_ref.avpr
@@ -8,7 +8,7 @@
"fields": [
{ "name":"name", "type": "string", "doc":"the name" },
{ "name": "value", "type": "string", "doc": "the value" },
- { "name": "type", "type": { "type": "enum", "name":"ValueType",
"symbols": ["JSON","BASE64BIN","PLAIN"] } }
+ { "name": "type", "type": { "type": "enum", "name":"ValueType",
"symbols": ["JSON","BASE64BIN","PLAIN"] }, "default": "PLAIN" }
]
}
],
diff --git a/lang/java/compiler/src/test/idl/output/simple.avpr
b/lang/java/compiler/src/test/idl/output/simple.avpr
index 438a4bd..5e86c6c 100644
--- a/lang/java/compiler/src/test/idl/output/simple.avpr
+++ b/lang/java/compiler/src/test/idl/output/simple.avpr
@@ -15,11 +15,6 @@
"symbols" : [ "A", "B", "C" ],
"default" : "C"
}, {
- "type" : "fixed",
- "name" : "MD5",
- "doc" : "An MD5 hash.",
- "size" : 16
- }, {
"type" : "record",
"name" : "TestRecord",
"doc" : "A TestRecord.",
@@ -43,7 +38,12 @@
"default" : "A"
}, {
"name" : "hash",
- "type" : "MD5",
+ "type" : {
+ "type" : "fixed",
+ "name" : "MD5",
+ "doc" : "An MD5 hash.",
+ "size" : 16
+ },
"default" : "0000000000000000"
}, {
"name" : "nullableHash",
