You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/01 15:39:41 UTC
[GitHub] [arrow] pitrou opened a new pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
pitrou opened a new pull request #8318:
URL: https://github.com/apache/arrow/pull/8318
Found by OSS-Fuzz. Should fix the following issues:
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=26064
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=26065
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=26067
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield closed pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
emkornfield closed pull request #8318:
URL: https://github.com/apache/arrow/pull/8318
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8318:
URL: https://github.com/apache/arrow/pull/8318#discussion_r498354560
##########
File path: cpp/src/parquet/arrow/schema.cc
##########
@@ -693,10 +693,12 @@ Status ApplyOriginalStorageMetadata(const Field& origin_field, SchemaField* infe
auto origin_type = origin_field.type();
auto inferred_type = inferred->field->type();
- if (inferred_type->id() == ::arrow::Type::TIMESTAMP) {
+ if (origin_type->id() == ::arrow::Type::TIMESTAMP &&
+ inferred_type->id() == ::arrow::Type::TIMESTAMP) {
// Restore time zone, if any
- const auto& ts_type = static_cast<const ::arrow::TimestampType&>(*inferred_type);
- const auto& ts_origin_type = static_cast<const ::arrow::TimestampType&>(*origin_type);
+ const auto& ts_type = checked_cast<const ::arrow::TimestampType&>(*inferred_type);
+ const auto& ts_origin_type =
+ checked_cast<const ::arrow::TimestampType&>(*origin_type);
Review comment:
Well, it's useful to have a debug mode check, IMHO.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8318:
URL: https://github.com/apache/arrow/pull/8318#issuecomment-702226179
https://issues.apache.org/jira/browse/ARROW-10150
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #8318:
URL: https://github.com/apache/arrow/pull/8318#discussion_r498356894
##########
File path: cpp/src/parquet/arrow/schema.cc
##########
@@ -693,10 +693,12 @@ Status ApplyOriginalStorageMetadata(const Field& origin_field, SchemaField* infe
auto origin_type = origin_field.type();
auto inferred_type = inferred->field->type();
- if (inferred_type->id() == ::arrow::Type::TIMESTAMP) {
+ if (origin_type->id() == ::arrow::Type::TIMESTAMP &&
+ inferred_type->id() == ::arrow::Type::TIMESTAMP) {
// Restore time zone, if any
- const auto& ts_type = static_cast<const ::arrow::TimestampType&>(*inferred_type);
- const auto& ts_origin_type = static_cast<const ::arrow::TimestampType&>(*origin_type);
+ const auto& ts_type = checked_cast<const ::arrow::TimestampType&>(*inferred_type);
+ const auto& ts_origin_type =
+ checked_cast<const ::arrow::TimestampType&>(*origin_type);
Review comment:
agreed, just wanted to make sure this wasn't the thing that actually solved the problem (but it couldn't really because it would throw an exception and still crash).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #8318:
URL: https://github.com/apache/arrow/pull/8318#issuecomment-702235798
+1 merging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #8318: ARROW-10150: [C++] Fix crashes on invalid Parquet file
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #8318:
URL: https://github.com/apache/arrow/pull/8318#discussion_r498352840
##########
File path: cpp/src/parquet/arrow/schema.cc
##########
@@ -693,10 +693,12 @@ Status ApplyOriginalStorageMetadata(const Field& origin_field, SchemaField* infe
auto origin_type = origin_field.type();
auto inferred_type = inferred->field->type();
- if (inferred_type->id() == ::arrow::Type::TIMESTAMP) {
+ if (origin_type->id() == ::arrow::Type::TIMESTAMP &&
+ inferred_type->id() == ::arrow::Type::TIMESTAMP) {
// Restore time zone, if any
- const auto& ts_type = static_cast<const ::arrow::TimestampType&>(*inferred_type);
- const auto& ts_origin_type = static_cast<const ::arrow::TimestampType&>(*origin_type);
+ const auto& ts_type = checked_cast<const ::arrow::TimestampType&>(*inferred_type);
+ const auto& ts_origin_type =
+ checked_cast<const ::arrow::TimestampType&>(*origin_type);
Review comment:
I thought checked_cast only had functionality in debug most and was essentially a static cast in prod? Or is this not the root cause of the issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org