Author: cutting
Date: Wed Oct 21 17:10:06 2009
New Revision: 828110
URL: http://svn.apache.org/viewvc?rev=828110&view=rev
Log:
AVRO-166. Improve error checking in Java schema parser. Contributed by Philip
Zeyliger.
Modified:
hadoop/avro/trunk/CHANGES.txt
hadoop/avro/trunk/src/java/org/apache/avro/Schema.java
hadoop/avro/trunk/src/test/java/org/apache/avro/io/TestValidatingIO.java
Modified: hadoop/avro/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/avro/trunk/CHANGES.txt?rev=828110&r1=828109&r2=828110&view=diff
==============================================================================
--- hadoop/avro/trunk/CHANGES.txt (original)
+++ hadoop/avro/trunk/CHANGES.txt Wed Oct 21 17:10:06 2009
@@ -20,6 +20,9 @@
AVRO-148. Add ant target to build c++ project. (sbanacho)
+ AVRO-166. Improve error checking in Java schema parser.
+ (Philip Zeyliger via cutting)
+
OPTIMIZATIONS
BUG FIXES
Modified: hadoop/avro/trunk/src/java/org/apache/avro/Schema.java
URL:
http://svn.apache.org/viewvc/hadoop/avro/trunk/src/java/org/apache/avro/Schema.java?rev=828110&r1=828109&r2=828110&view=diff
==============================================================================
--- hadoop/avro/trunk/src/java/org/apache/avro/Schema.java (original)
+++ hadoop/avro/trunk/src/java/org/apache/avro/Schema.java Wed Oct 21 17:10:06
2009
@@ -552,6 +552,8 @@
private final int size;
public FixedSchema(String name, String space, int size) {
super(Type.FIXED, name, space);
+ if (size < 0)
+ throw new IllegalArgumentException("Invalid fixed size: "+size);
this.size = size;
}
public int getFixedSize() { return size; }
@@ -689,21 +691,16 @@
throw new SchemaParseException("Undefined name: "+schema);
return result;
} else if (schema.isObject()) {
- JsonNode typeNode = schema.get("type");
- if (typeNode == null)
- throw new SchemaParseException("No type: "+schema);
- String type = typeNode.getTextValue();
+ String type = getRequiredText(schema, "type", "No type");
String name = null, space = null;
if (type.equals("record") || type.equals("error")
|| type.equals("enum") || type.equals("fixed")) {
- JsonNode nameNode = schema.get("name");
- name = nameNode != null ? nameNode.getTextValue() : null;
- JsonNode spaceNode = schema.get("namespace");
- space = spaceNode!=null?spaceNode.getTextValue():names.space();
+ name = getRequiredText(schema, "name", "No name in schema");
+ space = getOptionalText(schema, "namespace");
+ if (space == null)
+ space = names.space();
if (names.space() == null && space != null)
names.space(space); // set default namespace
- if (name == null)
- throw new SchemaParseException("No name in schema: "+schema);
}
if (type.equals("record") || type.equals("error")) { // record
LinkedHashMap<String,Field> fields = new LinkedHashMap<String,Field>();
@@ -745,8 +742,10 @@
} else if (type.equals("map")) { // map
return new MapSchema(parse(schema.get("values"), names));
} else if (type.equals("fixed")) { // fixed
- Schema result = new FixedSchema(name, space,
- schema.get("size").getIntValue());
+ JsonNode sizeNode = schema.get("size");
+ if (sizeNode == null || !sizeNode.isInt())
+ throw new SchemaParseException("Invalid or no size: "+schema);
+ Schema result = new FixedSchema(name, space, sizeNode.getIntValue());
if (name != null) names.add(result);
return result;
} else
@@ -761,6 +760,29 @@
}
}
+ /** Extracts text value associated to key from the container JsonNode,
+ * and throws {...@link SchemaParseException} if it doesn't exist.
+ *
+ * @param container Container where to find key.
+ * @param key Key to look for in container.
+ * @param error String to prepend to the SchemaParseException.
+ * @return
+ */
+ private static String getRequiredText(JsonNode container, String key,
+ String error) {
+ String out = getOptionalText(container, key);
+ if (null == out) {
+ throw new SchemaParseException(error + ": " + container);
+ }
+ return out;
+ }
+
+ /** Extracts text value associated to key from the container JsonNode. */
+ private static String getOptionalText(JsonNode container, String key) {
+ JsonNode jsonNode = container.get(key);
+ return jsonNode != null ? jsonNode.getTextValue() : null;
+ }
+
static JsonNode parseJson(String s) {
try {
return MAPPER.readTree(FACTORY.createJsonParser(new StringReader(s)));
Modified:
hadoop/avro/trunk/src/test/java/org/apache/avro/io/TestValidatingIO.java
URL:
http://svn.apache.org/viewvc/hadoop/avro/trunk/src/test/java/org/apache/avro/io/TestValidatingIO.java?rev=828110&r1=828109&r2=828110&view=diff
==============================================================================
--- hadoop/avro/trunk/src/test/java/org/apache/avro/io/TestValidatingIO.java
(original)
+++ hadoop/avro/trunk/src/test/java/org/apache/avro/io/TestValidatingIO.java
Wed Oct 21 17:10:06 2009
@@ -659,7 +659,7 @@
{ "\"string\"", "S10" },
{ "\"bytes\"", "b0" },
{ "\"bytes\"", "b10" },
- { "{\"type\":\"fixed\", \"name\":\"fi\", \"size\": 0}", "f0" },
+ { "{\"type\":\"fixed\", \"name\":\"fi\", \"size\": 1}", "f1" },
{ "{\"type\":\"fixed\", \"name\":\"fi\", \"size\": 10}", "f10" },
{ "{\"type\":\"enum\", \"name\":\"en\", \"symbols\":[\"v1\", \"v2\"]}",
"e1" },
