Skip to content

Commit

Permalink
Give better error information when trying to read an invalid Data3D w…
Browse files Browse the repository at this point in the history
…ith zero points

Instead of throwing ErrorInternal which is a bit misleading, throw a new exception ErrorData3DReadInvalidZeroRecords.

It looks like this:

trying to read an invalid Data3D with zero records - check for zero records before trying to read this Data3D section (ErrorInvalidZeroRecordsData3D): fileOffset cannot be 0; cvPathName=/data3D/0/points imageFileName=ZeroPointsInvalid.e57

Part of #262
  • Loading branch information
asmaloney committed Oct 22, 2023
1 parent f1c3940 commit aa90caa
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 7 deletions.
4 changes: 4 additions & 0 deletions include/E57Exception.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ namespace e57
/// passed an invalid value in Data3D pointFields
ErrorInvalidData3DValue = 52,

/// Older versions of this library (and E57RefImpl) incorrectly set the "fileOffset" to 0
/// when "recordCount" is 0. "fileOffset" must be greater than 0 (Table 9 in the standard).
ErrorData3DReadInvalidZeroRecords = 53,

/// @deprecated Will be removed in 4.0. Use e57::Success.
E57_SUCCESS DEPRECATED_ENUM( "Will be removed in 4.0. Use Success." ) = Success,
/// @deprecated Will be removed in 4.0. Use e57::ErrorBadCVHeader.
Expand Down
20 changes: 15 additions & 5 deletions src/CompressedVectorReaderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,17 +100,27 @@ namespace e57
//??? what if fault in this constructor?
cache_ = new PacketReadCache( imf->file_, 32 );

// Read CompressedVector section header
CompressedVectorSectionHeader sectionHeader;
// Check the file offset of this vector - it must be positive
uint64_t sectionLogicalStart = cVector_->getBinarySectionLogicalStart();
if ( sectionLogicalStart == 0 )
{
//??? should have caught this before got here, in XML read, get this if CV
// wasn't written to
// by writer.
// Older versions of this library (and E57RefImpl) incorrectly set the "fileOffset" to 0
// when "recordCount" is 0. "fileOffset" must be greater than 0 (Table 9 in the standard).
if ( maxRecordCount_ == 0 )
{
throw E57_EXCEPTION2( ErrorData3DReadInvalidZeroRecords,
"fileOffset cannot be 0; cvPathName=" + cVector_->pathName() +
" imageFileName=" + cVector_->imageFileName() );
}

//??? should have caught this before got here, in XML read, get this if CV wasn't written
// to by writer.
throw E57_EXCEPTION2( ErrorInternal, "imageFileName=" + cVector_->imageFileName() +
" cvPathName=" + cVector_->pathName() );
}

// Read CompressedVector section header
CompressedVectorSectionHeader sectionHeader;
imf->file_->seek( sectionLogicalStart, CheckedFile::Logical );
imf->file_->read( reinterpret_cast<char *>( &sectionHeader ), sizeof( sectionHeader ) );

Expand Down
7 changes: 5 additions & 2 deletions src/E57Exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,12 @@ namespace e57
case ErrorInvarianceViolation:
return "class invariance constraint violation in debug mode (ErrorInvarianceViolation)";
case ErrorInvalidNodeType:
return "an invalid node type was passed in Data3D pointFields";
return "an invalid node type was passed in Data3D pointFields (ErrorInvalidNodeType)";
case ErrorInvalidData3DValue:
return "an invalid value was passed in Data3D pointFields";
return "an invalid value was passed in Data3D pointFields (ErrorInvalidData3DValue)";
case ErrorData3DReadInvalidZeroRecords:
return "trying to read an invalid Data3D with zero records - check for zero records "
"before trying to read this Data3D section (ErrorInvalidZeroRecordsData3D)";

default:
return "unknown error (" + std::to_string( ecode ) + ")";
Expand Down
32 changes: 32 additions & 0 deletions test/src/test_SimpleReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,38 @@ TEST( SimpleReaderData, Empty )
delete reader;
}

TEST( SimpleReaderData, ZeroPointsInvalid )
{
e57::Reader *reader = nullptr;

E57_ASSERT_NO_THROW(
reader = new e57::Reader( TestData::Path() + "/self/ZeroPointsInvalid.e57", {} ) );

ASSERT_TRUE( reader->IsOpen() );
EXPECT_EQ( reader->GetImage2DCount(), 0 );
EXPECT_EQ( reader->GetData3DCount(), 1 );

e57::E57Root fileHeader;
ASSERT_TRUE( reader->GetE57Root( fileHeader ) );

CheckFileHeader( fileHeader );
EXPECT_EQ( fileHeader.guid, "{EC1A0DE4-F76F-44CE-E527-789EEB818347}" );

e57::Data3D data3DHeader;
ASSERT_TRUE( reader->ReadData3D( 0, data3DHeader ) );

ASSERT_EQ( data3DHeader.pointCount, 0 );

const uint64_t cNumPoints = data3DHeader.pointCount;

e57::Data3DPointsFloat pointsData( data3DHeader );

E57_ASSERT_THROW( auto vectorReader =
reader->SetUpData3DPointsData( 0, cNumPoints, pointsData ); );

delete reader;
}

TEST( SimpleReaderData, BadCRC )
{
E57_ASSERT_THROW( e57::Reader( TestData::Path() + "/self/bad-crc.e57", {} ) );
Expand Down

0 comments on commit aa90caa

Please sign in to comment.