You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@avro.apache.org by GitBox <gi...@apache.org> on 2022/07/12 12:45:36 UTC
[GitHub] [avro] github-code-scanning[bot] commented on a diff in pull request #1768: Avro 530 def recursion
github-code-scanning[bot] commented on code in PR #1768:
URL: https://github.com/apache/avro/pull/1768#discussion_r918924967
##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -694,6 +697,81 @@
public String toString() {
return name + " type:" + schema.type + " pos:" + position;
}
+
+ static Field parse(JsonNode field, Names names, String namespace) {
+ String fieldName = getRequiredText(field, "name", "No field name");
+ String fieldDoc = getOptionalText(field, "doc");
+ JsonNode fieldTypeNode = field.get("type");
+ if (fieldTypeNode == null) {
+ throw new SchemaParseException("No field type: " + field);
+ }
+
+ Schema fieldSchema = null;
+ if (fieldTypeNode.isTextual()) {
+ Schema schemaField = names.get(fieldTypeNode.textValue());
+ if (schemaField == null) {
+ schemaField = names.get(namespace + "." + fieldTypeNode.textValue());
+ }
+ if (schemaField == null) {
+ throw new SchemaParseException(fieldTypeNode + " is not a defined name." + " The type of the \"" + fieldName
+ + "\" field must be a defined name or a {\"type\": ...} expression.");
+ }
+ fieldSchema = schemaField;
+ } else if (fieldTypeNode.isObject()) {
+ fieldSchema = resolveSchema(fieldTypeNode, names, namespace);
+ if (fieldSchema == null) {
+ fieldSchema = Schema.parseFieldsOnly(fieldTypeNode, names, namespace);
+ }
+ } else if (fieldTypeNode.isArray()) {
+ List<Schema> unionTypes = new ArrayList<>();
+
+ fieldTypeNode.forEach((JsonNode node) -> {
+ Schema subSchema = null;
+ if (node.isTextual()) {
+ subSchema = names.get(node.asText());
+ if (subSchema == null) {
+ subSchema = names.get(namespace + "." + node.asText());
+ }
+ } else if (node.isObject()) {
+ subSchema = Schema.parseFieldsOnly(node, names, namespace);
+ } else {
+ throw new SchemaParseException("Illegal type in union : " + node);
+ }
+ if (subSchema == null) {
+ throw new SchemaParseException("Null element in union : " + node);
+ }
+ unionTypes.add(subSchema);
+ });
+
+ fieldSchema = Schema.createUnion(unionTypes);
+ }
+
+ if (fieldSchema == null) {
+ throw new SchemaParseException("Can't find type for field " + fieldName);
+ }
+ Field.Order order = Field.Order.ASCENDING;
+ JsonNode orderNode = field.get("order");
+ if (orderNode != null)
+ order = Field.Order.valueOf(orderNode.textValue().toUpperCase(Locale.ENGLISH));
+ JsonNode defaultValue = field.get("default");
+
+ if (defaultValue != null && fieldSchema != null
+ && (Type.FLOAT.equals(fieldSchema.getType()) || Type.DOUBLE.equals(fieldSchema.getType()))
+ && defaultValue.isTextual())
+ defaultValue = new DoubleNode(Double.valueOf(defaultValue.textValue()));
Review Comment:
## Missing catch of NumberFormatException
Potential uncaught 'java.lang.NumberFormatException'.
[Show more details](https://github.com/apache/avro/security/code-scanning/2776)
##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -694,6 +697,81 @@
public String toString() {
return name + " type:" + schema.type + " pos:" + position;
}
+
+ static Field parse(JsonNode field, Names names, String namespace) {
+ String fieldName = getRequiredText(field, "name", "No field name");
+ String fieldDoc = getOptionalText(field, "doc");
+ JsonNode fieldTypeNode = field.get("type");
+ if (fieldTypeNode == null) {
+ throw new SchemaParseException("No field type: " + field);
+ }
+
+ Schema fieldSchema = null;
+ if (fieldTypeNode.isTextual()) {
+ Schema schemaField = names.get(fieldTypeNode.textValue());
+ if (schemaField == null) {
+ schemaField = names.get(namespace + "." + fieldTypeNode.textValue());
+ }
+ if (schemaField == null) {
+ throw new SchemaParseException(fieldTypeNode + " is not a defined name." + " The type of the \"" + fieldName
+ + "\" field must be a defined name or a {\"type\": ...} expression.");
+ }
+ fieldSchema = schemaField;
+ } else if (fieldTypeNode.isObject()) {
+ fieldSchema = resolveSchema(fieldTypeNode, names, namespace);
+ if (fieldSchema == null) {
+ fieldSchema = Schema.parseFieldsOnly(fieldTypeNode, names, namespace);
+ }
+ } else if (fieldTypeNode.isArray()) {
+ List<Schema> unionTypes = new ArrayList<>();
+
+ fieldTypeNode.forEach((JsonNode node) -> {
+ Schema subSchema = null;
+ if (node.isTextual()) {
+ subSchema = names.get(node.asText());
+ if (subSchema == null) {
+ subSchema = names.get(namespace + "." + node.asText());
+ }
+ } else if (node.isObject()) {
+ subSchema = Schema.parseFieldsOnly(node, names, namespace);
+ } else {
+ throw new SchemaParseException("Illegal type in union : " + node);
+ }
+ if (subSchema == null) {
+ throw new SchemaParseException("Null element in union : " + node);
+ }
+ unionTypes.add(subSchema);
+ });
+
+ fieldSchema = Schema.createUnion(unionTypes);
+ }
+
+ if (fieldSchema == null) {
+ throw new SchemaParseException("Can't find type for field " + fieldName);
+ }
+ Field.Order order = Field.Order.ASCENDING;
+ JsonNode orderNode = field.get("order");
+ if (orderNode != null)
+ order = Field.Order.valueOf(orderNode.textValue().toUpperCase(Locale.ENGLISH));
+ JsonNode defaultValue = field.get("default");
+
+ if (defaultValue != null && fieldSchema != null
Review Comment:
## Useless null check
This check is useless, [fieldSchema](1) cannot be null here, since it is guarded by [... == ...](2).
[Show more details](https://github.com/apache/avro/security/code-scanning/2773)
##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -1668,78 +1746,38 @@
}
}
- /** @see #parse(String) */
- static Schema parse(JsonNode schema, Names names) {
+ static Schema parseNamesDeclared(JsonNode schema, Names names, String nameSpace) {
if (schema == null) {
- throw new SchemaParseException("Cannot parse <null> schema");
+ return null;
}
- if (schema.isTextual()) { // name
- Schema result = names.get(schema.textValue());
- if (result == null)
- throw new SchemaParseException("Undefined name: " + schema);
- return result;
- } else if (schema.isObject()) {
- Schema result;
- String type = getRequiredText(schema, "type", "No type");
+ if (schema.isObject()) {
+
+ String type = Schema.getOptionalText(schema, "type");
Name name = null;
- String savedSpace = names.space();
+
String doc = null;
+ Schema result = null;
final boolean isTypeError = "error".equals(type);
final boolean isTypeRecord = "record".equals(type);
final boolean isTypeEnum = "enum".equals(type);
final boolean isTypeFixed = "fixed".equals(type);
+
if (isTypeRecord || isTypeError || isTypeEnum || isTypeFixed) {
String space = getOptionalText(schema, "namespace");
doc = getOptionalText(schema, "doc");
if (space == null)
- space = savedSpace;
+ space = nameSpace;
name = new Name(getRequiredText(schema, "name", "No name in schema"), space);
- names.space(name.space); // set default namespace
}
- if (PRIMITIVES.containsKey(type)) { // primitive
- result = create(PRIMITIVES.get(type));
- } else if (isTypeRecord || isTypeError) { // record
- List<Field> fields = new ArrayList<>();
+ if (isTypeRecord || isTypeError) { // record
result = new RecordSchema(name, doc, isTypeError);
- if (name != null)
- names.add(result);
+ names.add(result);
JsonNode fieldsNode = schema.get("fields");
+
if (fieldsNode == null || !fieldsNode.isArray())
throw new SchemaParseException("Record has no fields: " + schema);
- for (JsonNode field : fieldsNode) {
- String fieldName = getRequiredText(field, "name", "No field name");
- String fieldDoc = getOptionalText(field, "doc");
- JsonNode fieldTypeNode = field.get("type");
- if (fieldTypeNode == null)
- throw new SchemaParseException("No field type: " + field);
- if (fieldTypeNode.isTextual() && names.get(fieldTypeNode.textValue()) == null)
- throw new SchemaParseException(fieldTypeNode + " is not a defined name." + " The type of the \"" + fieldName
- + "\" field must be a defined name or a {\"type\": ...} expression.");
- Schema fieldSchema = parse(fieldTypeNode, names);
- Field.Order order = Field.Order.ASCENDING;
- JsonNode orderNode = field.get("order");
- if (orderNode != null)
- order = Field.Order.valueOf(orderNode.textValue().toUpperCase(Locale.ENGLISH));
- JsonNode defaultValue = field.get("default");
- if (defaultValue != null
- && (Type.FLOAT.equals(fieldSchema.getType()) || Type.DOUBLE.equals(fieldSchema.getType()))
- && defaultValue.isTextual())
- defaultValue = new DoubleNode(Double.valueOf(defaultValue.textValue()));
- Field f = new Field(fieldName, fieldSchema, fieldDoc, defaultValue, true, order);
- Iterator<String> i = field.fieldNames();
- while (i.hasNext()) { // add field props
- String prop = i.next();
- if (!FIELD_RESERVED.contains(prop))
- f.addProp(prop, field.get(prop));
- }
- f.aliases = parseAliases(field);
- fields.add(f);
- if (fieldSchema.getLogicalType() == null && getOptionalText(field, LOGICAL_TYPE_PROP) != null)
- LOG.warn(
- "Ignored the {}.{}.logicalType property (\"{}\"). It should probably be nested inside the \"type\" for the field.",
- name, fieldName, getOptionalText(field, "logicalType"));
- }
- result.setFields(fields);
+ exploreFields(fieldsNode, names, name.space);
Review Comment:
## Dereferenced variable may be null
Variable [name](1) may be null here because of [this](2) assignment.
[Show more details](https://github.com/apache/avro/security/code-scanning/2774)
##########
lang/java/avro/src/test/java/org/apache/avro/TestProtocol.java:
##########
@@ -23,10 +23,33 @@
import static java.util.Collections.singletonMap;
import static org.junit.Assert.*;
+import java.io.File;
+import java.io.IOException;
+
+import org.junit.Assert;
import org.junit.Test;
public class TestProtocol {
+ @Test
+ public void parse() throws IOException {
+ File fic = new File("../../../share/test/schemas/namespace.avpr");
+ Protocol protocol = Protocol.parse(fic);
+ Assert.assertNotNull(protocol);
+ Assert.assertEquals("TestNamespace", protocol.getName());
+ }
+
+ @Test
+ public void testProUnknown() {
+ String userStatus = "{ \"protocol\" : \"p1\", " + "\"types\": ["
+ + "{\"name\": \"User\", \"type\": \"record\", \"fields\": [{\"name\": \"current_status\", \"type\": \"Status\"}]},\n"
+ + "\n"
+ + "{\"name\": \"Status\", \"type\": \"record\", \"fields\": [{\"name\": \"author\", \"type\": \"User\"}]}"
+ + "]}";
+
+ Protocol protocol = Protocol.parse(userStatus);
Review Comment:
## Unread local variable
Variable 'Protocol protocol' is never read.
[Show more details](https://github.com/apache/avro/security/code-scanning/2775)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org