You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@orc.apache.org by md...@apache.org on 2018/03/21 15:41:32 UTC
orc git commit: ORC-313: Check subtype count of LIST,
MAP and UNION types
Repository: orc
Updated Branches:
refs/heads/master 8fae89ff4 -> b6d360d1a
ORC-313: Check subtype count of LIST, MAP and UNION types
Fixes #229
Signed-off-by: Deepak Majeti <md...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/orc/repo
Commit: http://git-wip-us.apache.org/repos/asf/orc/commit/b6d360d1
Tree: http://git-wip-us.apache.org/repos/asf/orc/tree/b6d360d1
Diff: http://git-wip-us.apache.org/repos/asf/orc/diff/b6d360d1
Branch: refs/heads/master
Commit: b6d360d1ab19f5847b137fac90a28c934dab1298
Parents: 8fae89f
Author: stiga-huang <hu...@gmail.com>
Authored: Sat Mar 3 01:21:23 2018 -0800
Committer: Deepak Majeti <md...@apache.org>
Committed: Wed Mar 21 11:41:00 2018 -0400
----------------------------------------------------------------------
c++/src/TypeImpl.cc | 6 ++
c++/test/TestType.cc | 42 +++++++++++++
java/core/src/java/org/apache/orc/OrcUtils.java | 15 ++++-
.../test/org/apache/orc/TestCorruptTypes.java | 63 ++++++++++++++++++++
.../java/org/apache/orc/tools/PrintData.java | 1 +
5 files changed, 126 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/orc/blob/b6d360d1/c++/src/TypeImpl.cc
----------------------------------------------------------------------
diff --git a/c++/src/TypeImpl.cc b/c++/src/TypeImpl.cc
index f9e4cd6..63a5038 100644
--- a/c++/src/TypeImpl.cc
+++ b/c++/src/TypeImpl.cc
@@ -379,6 +379,12 @@ namespace orc {
case proto::Type_Kind_MAP:
case proto::Type_Kind_UNION: {
TypeImpl* result = new TypeImpl(static_cast<TypeKind>(type.kind()));
+ if (type.kind() == proto::Type_Kind_LIST && type.subtypes_size() != 1)
+ throw ParseError("Illegal LIST type that doesn't contain one subtype");
+ if (type.kind() == proto::Type_Kind_MAP && type.subtypes_size() != 2)
+ throw ParseError("Illegal MAP type that doesn't contain two subtypes");
+ if (type.kind() == proto::Type_Kind_UNION && type.subtypes_size() == 0)
+ throw ParseError("Illegal UNION type that doesn't contain any subtypes");
for(int i=0; i < type.subtypes_size(); ++i) {
result->addUnionChild(convertType(footer.types(static_cast<int>
(type.subtypes(i))),
http://git-wip-us.apache.org/repos/asf/orc/blob/b6d360d1/c++/test/TestType.cc
----------------------------------------------------------------------
diff --git a/c++/test/TestType.cc b/c++/test/TestType.cc
index 8ce9313..d23c6b7 100644
--- a/c++/test/TestType.cc
+++ b/c++/test/TestType.cc
@@ -18,6 +18,7 @@
#include "Adaptor.hh"
#include "OrcTest.hh"
+#include "orc/Exceptions.hh"
#include "orc/Type.hh"
#include "wrap/gtest-wrapper.h"
@@ -297,4 +298,45 @@ namespace orc {
type = Type::buildTypeFromString(typeStr);
EXPECT_EQ(typeStr, type->toString());
}
+
+ void testCorruptHelper(const proto::Type& type,
+ const proto::Footer& footer,
+ const char* errMsg) {
+ try {
+ convertType(type, footer);
+ FAIL() << "Should throw ParseError for ill types";
+ } catch (ParseError& e) {
+ EXPECT_EQ(e.what(), std::string(errMsg));
+ } catch (...) {
+ FAIL() << "Should only throw ParseError for ill types";
+ }
+ }
+
+ TEST(TestType, testCorruptNestType) {
+ proto::Footer footer; // not used
+
+ proto::Type illListType;
+ illListType.set_kind(proto::Type_Kind_LIST);
+ testCorruptHelper(illListType, footer,
+ "Illegal LIST type that doesn't contain one subtype");
+ illListType.add_subtypes(2);
+ illListType.add_subtypes(3);
+ testCorruptHelper(illListType, footer,
+ "Illegal LIST type that doesn't contain one subtype");
+
+ proto::Type illMapType;
+ illMapType.set_kind(proto::Type_Kind_MAP);
+ illMapType.add_subtypes(2);
+ testCorruptHelper(illMapType, footer,
+ "Illegal MAP type that doesn't contain two subtypes");
+ illMapType.add_subtypes(3);
+ illMapType.add_subtypes(4);
+ testCorruptHelper(illMapType, footer,
+ "Illegal MAP type that doesn't contain two subtypes");
+
+ proto::Type illUnionType;
+ illUnionType.set_kind(proto::Type_Kind_UNION);
+ testCorruptHelper(illUnionType, footer,
+ "Illegal UNION type that doesn't contain any subtypes");
+ }
}
http://git-wip-us.apache.org/repos/asf/orc/blob/b6d360d1/java/core/src/java/org/apache/orc/OrcUtils.java
----------------------------------------------------------------------
diff --git a/java/core/src/java/org/apache/orc/OrcUtils.java b/java/core/src/java/org/apache/orc/OrcUtils.java
index 37e624c..604660a 100644
--- a/java/core/src/java/org/apache/orc/OrcUtils.java
+++ b/java/core/src/java/org/apache/orc/OrcUtils.java
@@ -458,7 +458,8 @@ public class OrcUtils {
*/
public static
TypeDescription convertTypeFromProtobuf(List<OrcProto.Type> types,
- int rootColumn) {
+ int rootColumn)
+ throws FileFormatException {
OrcProto.Type type = types.get(rootColumn);
switch (type.getKind()) {
case BOOLEAN:
@@ -503,9 +504,17 @@ public class OrcUtils {
return result;
}
case LIST:
+ if (type.getSubtypesCount() != 1) {
+ throw new FileFormatException("LIST type should contain exactly " +
+ "one subtype but has " + type.getSubtypesCount());
+ }
return TypeDescription.createList(
convertTypeFromProtobuf(types, type.getSubtypes(0)));
case MAP:
+ if (type.getSubtypesCount() != 2) {
+ throw new FileFormatException("MAP type should contain exactly " +
+ "two subtypes but has " + type.getSubtypesCount());
+ }
return TypeDescription.createMap(
convertTypeFromProtobuf(types, type.getSubtypes(0)),
convertTypeFromProtobuf(types, type.getSubtypes(1)));
@@ -518,6 +527,10 @@ public class OrcUtils {
return result;
}
case UNION: {
+ if (type.getSubtypesCount() == 0) {
+ throw new FileFormatException("UNION type should contain at least" +
+ " one subtype but has none");
+ }
TypeDescription result = TypeDescription.createUnion();
for(int f=0; f < type.getSubtypesCount(); ++f) {
result.addUnionChild(
http://git-wip-us.apache.org/repos/asf/orc/blob/b6d360d1/java/core/src/test/org/apache/orc/TestCorruptTypes.java
----------------------------------------------------------------------
diff --git a/java/core/src/test/org/apache/orc/TestCorruptTypes.java b/java/core/src/test/org/apache/orc/TestCorruptTypes.java
new file mode 100644
index 0000000..8808a08
--- /dev/null
+++ b/java/core/src/test/org/apache/orc/TestCorruptTypes.java
@@ -0,0 +1,63 @@
+/**
+ * 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
+ *
+ * http://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.orc;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.fail;
+
+public class TestCorruptTypes {
+
+ @Test
+ public void testIllType() {
+ testCorruptHelper(OrcProto.Type.Kind.LIST, 0,
+ "LIST type should contain exactly one subtype but has 0");
+ testCorruptHelper(OrcProto.Type.Kind.LIST, 2,
+ "LIST type should contain exactly one subtype but has 2");
+ testCorruptHelper(OrcProto.Type.Kind.MAP, 1,
+ "MAP type should contain exactly two subtypes but has 1");
+ testCorruptHelper(OrcProto.Type.Kind.MAP, 3,
+ "MAP type should contain exactly two subtypes but has 3");
+ testCorruptHelper(OrcProto.Type.Kind.UNION, 0,
+ "UNION type should contain at least one subtype but has none");
+ }
+
+ private void testCorruptHelper(OrcProto.Type.Kind type,
+ int subTypesCnt,
+ String errMsg) {
+
+ List<OrcProto.Type> types = new ArrayList<OrcProto.Type>();
+ OrcProto.Type.Builder builder = OrcProto.Type.newBuilder().setKind(type);
+ for (int i = 0; i < subTypesCnt; ++i) {
+ builder.addSubtypes(i + 2);
+ }
+ types.add(builder.build());
+ try {
+ OrcUtils.convertTypeFromProtobuf(types, 0);
+ fail("Should throw FileFormatException for ill types");
+ } catch (FileFormatException e) {
+ assertEquals(errMsg, e.getMessage());
+ } catch (Throwable e) {
+ fail("Should only trow FileFormatException for ill types");
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/orc/blob/b6d360d1/java/tools/src/java/org/apache/orc/tools/PrintData.java
----------------------------------------------------------------------
diff --git a/java/tools/src/java/org/apache/orc/tools/PrintData.java b/java/tools/src/java/org/apache/orc/tools/PrintData.java
index ebd9ae1..5d74a21 100644
--- a/java/tools/src/java/org/apache/orc/tools/PrintData.java
+++ b/java/tools/src/java/org/apache/orc/tools/PrintData.java
@@ -242,6 +242,7 @@ public class PrintData {
System.out.println(FileDump.SEPARATOR);
} catch (Exception e) {
System.err.println("Unable to dump data for file: " + file);
+ e.printStackTrace();
continue;
}
}