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 <[email protected]> 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 <[email protected]> Authored: Sat Mar 3 01:21:23 2018 -0800 Committer: Deepak Majeti <[email protected]> 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; } }
