You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by stiga-huang <gi...@git.apache.org> on 2018/03/15 23:41:38 UTC
[GitHub] orc pull request #231: ORC-317: [C++] check that indices in the protobuf typ...
GitHub user stiga-huang opened a pull request:
https://github.com/apache/orc/pull/231
ORC-317: [C++] check that indices in the protobuf type tree are valid
A corrupt file may contain loops in the protobuf type tree. We should check this before invoking `convertType`. The file attached to the Jira can reproduce this bug. Tests are added for coverage.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stiga-huang/orc check-type-id
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/orc/pull/231.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #231
----
commit edece2ba16da09433fa7ca5f5cfeb966df09d58a
Author: stiga-huang <hu...@...>
Date: 2018-03-15T23:37:55Z
ORC-317: check that indices in the protobuf type tree are valid
----
---
[GitHub] orc issue #231: ORC-317: [C++] check that indices in the protobuf type tree ...
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on the issue:
https://github.com/apache/orc/pull/231
Actually, I just committed it. Thanks!
I did adjust the comment on the code to make it more consistent and remove the jira reference. The git history does that better if we need to track the history.
---
[GitHub] orc pull request #231: ORC-317: [C++] check that indices in the protobuf typ...
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on a diff in the pull request:
https://github.com/apache/orc/pull/231#discussion_r175949471
--- Diff: c++/src/Reader.cc ---
@@ -897,6 +897,27 @@ namespace orc {
return REDUNDANT_MOVE(postscript);
}
+ // ORC-317: check that indices in the type tree are valid, so we won't crash
+ // when we convert the proto::Types to TypeImpls.
+ void checkProtoTypeIds(int &index, const proto::Footer &footer) {
--- End diff --
I'd suggest only passing in the footer here and check all of the types in a loop.
---
[GitHub] orc pull request #231: ORC-317: [C++] check that indices in the protobuf typ...
Posted by stiga-huang <gi...@git.apache.org>.
Github user stiga-huang commented on a diff in the pull request:
https://github.com/apache/orc/pull/231#discussion_r176673942
--- Diff: c++/src/Reader.cc ---
@@ -897,6 +897,27 @@ namespace orc {
return REDUNDANT_MOVE(postscript);
}
+ // ORC-317: check that indices in the type tree are valid, so we won't crash
+ // when we convert the proto::Types to TypeImpls.
+ void checkProtoTypeIds(int &index, const proto::Footer &footer) {
+ if (index >= footer.types_size())
+ throw ParseError(std::string("Footer is corrupt that it lost types(") +
+ std::to_string(index) + ")");
+ const proto::Type& type = footer.types(index);
+
+ int origin_index = index;
+ for (int i = 0; i < type.subtypes_size(); ++i) {
+ int proto_index = static_cast<int>(type.subtypes(i));
--- End diff --
That's cool!
---
[GitHub] orc issue #231: ORC-317: [C++] check that indices in the protobuf type tree ...
Posted by stiga-huang <gi...@git.apache.org>.
Github user stiga-huang commented on the issue:
https://github.com/apache/orc/pull/231
@omalley @majetideepak could you have a look at these? We need this bug be fixed to integrate the c++ library into Impala (ORC-315).
---
[GitHub] orc pull request #231: ORC-317: [C++] check that indices in the protobuf typ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/orc/pull/231
---
[GitHub] orc pull request #231: ORC-317: [C++] check that indices in the protobuf typ...
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on a diff in the pull request:
https://github.com/apache/orc/pull/231#discussion_r175950941
--- Diff: c++/src/Reader.cc ---
@@ -897,6 +897,27 @@ namespace orc {
return REDUNDANT_MOVE(postscript);
}
+ // ORC-317: check that indices in the type tree are valid, so we won't crash
+ // when we convert the proto::Types to TypeImpls.
+ void checkProtoTypeIds(int &index, const proto::Footer &footer) {
+ if (index >= footer.types_size())
+ throw ParseError(std::string("Footer is corrupt that it lost types(") +
+ std::to_string(index) + ")");
+ const proto::Type& type = footer.types(index);
+
+ int origin_index = index;
+ for (int i = 0; i < type.subtypes_size(); ++i) {
+ int proto_index = static_cast<int>(type.subtypes(i));
--- End diff --
For each type:
- For each subtype value:
+ ensure it is larger than the parent type id
+ ensure it is larger than the previous subtype
+ ensure it is lower or equal to than the max type id
That should prevent any failures, even with malformed ORC file footers.
---