ORC-317: [C++] check that indices in the protobuf type tree are valid #231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@omalley @majetideepak could you have a look at these? We need this bug be fixed to integrate the c++ library into Impala (ORC-315). |
omalley
requested changes
Mar 20, 2018
c++/src/Reader.cc
Outdated
| @@ -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) { | |||
There was a problem hiding this comment.
I'd suggest only passing in the footer here and check all of the types in a loop.
c++/src/Reader.cc
Outdated
|
|
||
| int origin_index = index; | ||
| for (int i = 0; i < type.subtypes_size(); ++i) { | ||
| int proto_index = static_cast<int>(type.subtypes(i)); |
There was a problem hiding this comment.
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.
stiga-huang
force-pushed
the
check-type-id
branch
from
edece2b
to
8107d12
Compare
majetideepak
approved these changes
Mar 23, 2018
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.