Give better error information when trying to read an invalid Data3D w…

…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

include/E57Exception.h

@@ -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.

src/CompressedVectorReaderImpl.cpp

@@ -97,20 +97,27 @@ namespace e57
 
       ImageFileImplSharedPtr imf( cVector_->destImageFile_ );
 
-      //??? 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 ) );
 
@@ -125,6 +132,9 @@ namespace e57
       uint64_t dataLogicalOffset =
          imf->file_->physicalToLogical( sectionHeader.dataPhysicalOffset );
 
+      //??? what if fault in this constructor?
+      cache_ = new PacketReadCache( imf->file_, 32 );
+
       // Verify that packet given by dataPhysicalOffset is actually a data packet,
       // init channels
       {

src/E57Exception.cpp

@@ -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 ) + ")";

test/src/test_SimpleReader.cpp

@@ -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", {} ) );