vtkF3DQuakeMDLImporter: Add CanReadFile support

Author: mwestphal

plugins/native/module/Testing/CMakeLists.txt

@@ -7,6 +7,7 @@ if(VTK_VERSION VERSION_GREATER_EQUAL 9.4.20250501)
     TestF3DSplatReader.cxx
     TestF3DPLYReader.cxx
     TestF3DQuakeMDLImporterStream.cxx
+    TestF3DQuakeMDLImporterStreamError.cxx
     TestF3DQuakeMDLImporterInexistent.cxx
     TestF3DQuakeMDLParser.cxx
   )

plugins/native/module/Testing/TestF3DQuakeMDLImporterStream.cxx

@@ -19,6 +19,12 @@ int TestF3DQuakeMDLImporterStream(int vtkNotUsed(argc), char* argv[])
     return EXIT_FAILURE;
   }
 
+  if (!vtkF3DQuakeMDLImporter::CanReadFile(stream))
+  {
+    std::cerr << "Unexpected CanReadFile failure\n";
+    return EXIT_FAILURE;
+  }
+
   vtkNew<vtkF3DQuakeMDLImporter> importer;
   importer->SetStream(stream);
   importer->Update();

plugins/native/module/Testing/TestF3DQuakeMDLImporterStreamError.cxx

@@ -0,0 +1,49 @@
+#include <vtkDataSet.h>
+#include <vtkFileResourceStream.h>
+#include <vtkMapper.h>
+#include <vtkNew.h>
+#include <vtkRenderer.h>
+#include <vtkTestUtilities.h>
+
+#include "vtkF3DQuakeMDLImporter.h"
+
+#include <iostream>
+
+int TestF3DQuakeMDLImporterStreamError(int vtkNotUsed(argc), char* argv[])
+{
+  if (vtkF3DQuakeMDLImporter::CanReadFile(nullptr))
+  {
+    std::cerr << "Unexpected CanReadFile success with nullptr\n";
+    return EXIT_FAILURE;
+  }
+
+  vtkNew<vtkFileResourceStream> stream;
+  std::string path = std::string(argv[1]) + "data/invalid.mdl";
+  if (!stream->Open(path.c_str()))
+  {
+    std::cerr << "Cannot open file\n";
+    return EXIT_FAILURE;
+  }
+
+  if (vtkF3DQuakeMDLImporter::CanReadFile(stream))
+  {
+    std::cerr << "Unexpected CanReadFile success invalid file\n";
+    return EXIT_FAILURE;
+  }
+
+  path = std::string(argv[1]) + "data/w_medkit_hl.mdl";
+  if (!stream->Open(path.c_str()))
+  {
+    std::cerr << "Cannot open file\n";
+    return EXIT_FAILURE;
+  }
+
+  if (vtkF3DQuakeMDLImporter::CanReadFile(stream))
+  {
+    std::cerr << "Unexpected CanReadFile success wrong version file\n";
+    return EXIT_FAILURE;
+  }
+
+  // No need to read invalid file, already covered
+  return EXIT_SUCCESS;
+}

plugins/native/module/vtkF3DQuakeMDLImporter.cxx

@@ -156,9 +156,49 @@ struct vtkF3DQuakeMDLImporter::vtkInternals
     return reinterpret_cast<const mdl_simpleframe_t*>(buffer.data() + offset);
   }
 
+  //----------------------------------------------------------------------------
+  static bool ReadAndCheckHeader(const std::vector<uint8_t>& buffer, vtkObject* object,
+    size_t& offset, const mdl_header_t*& header)
+  {
+    // Read header
+    try
+    {
+      header = vtkInternals::ReadFromVector<mdl_header_t>(buffer, offset);
+    }
+    catch (const F3DRangeError&)
+    {
+      if (object)
+      {
+        vtkErrorWithObjectMacro(object, "Invalid MDL file");
+      }
+      return false;
+    }
+
+    // Check magic number for "IPDO" or "IDST"
+    if (!(header->ident == 0x4F504449 || header->ident == 0x54534449))
+    {
+      if (object)
+      {
+        vtkErrorWithObjectMacro(object, "Incompatible MDL header");
+      }
+      return false;
+    }
+
+    // Check version for v6 exactly
+    if (header->version != 6)
+    {
+      if (object)
+      {
+        vtkErrorWithObjectMacro(object, "Unsupported MDL version. Only version 6 is supported");
+      }
+      return false;
+    }
+    return true;
+  }
+
   //----------------------------------------------------------------------------
   // Safer buffer typecasting with auto offset incrementing with variable length data
-  const mdl_simpleframe_t* ReadFromVectorSimpleframe(
+  static const mdl_simpleframe_t* ReadFromVectorSimpleframe(
     const std::vector<uint8_t>& buffer, size_t& offset, size_t num_verts = 0)
   {
     static constexpr auto mdl_simpleframe_t_fixed_size =
@@ -563,34 +603,15 @@ struct vtkF3DQuakeMDLImporter::vtkInternals
     stream->Seek(0, vtkResourceStream::SeekDirection::Begin);
 
     // Read stream into buffer
+    // TODO rework to avoid copying the whole buffer
     std::vector<uint8_t> buffer(length);
     stream->Read(buffer.data(), length);
 
-    // Read header
-    // XXX: This is completely unsafe, should be rewritten using modern API
+    // Read Header
     size_t offset = 0;
     const mdl_header_t* header;
-    try
-    {
-      header = vtkInternals::ReadFromVector<mdl_header_t>(buffer, offset);
-    }
-    catch (const F3DRangeError&)
-    {
-      vtkErrorWithObjectMacro(this->Parent, "Invalid MDL file");
-      return false;
-    }
-
-    // Check magic number for "IPDO" or "IDST"
-    if (!(header->ident == 0x4F504449 || header->ident == 0x54534449))
+    if (!vtkInternals::ReadAndCheckHeader(buffer, this->Parent, offset, header))
     {
-      vtkErrorWithObjectMacro(this->Parent, "Incompatible MDL header");
-      return false;
-    }
-
-    // Check version for v6 exactly
-    if (header->version != 6)
-    {
-      vtkErrorWithObjectMacro(this->Parent, "Unsupported MDL version. Only version 6 is supported");
       return false;
     }
 
@@ -791,3 +812,30 @@ bool vtkF3DQuakeMDLImporter::GetTemporalInformation(
   }
   return true;
 }
+
+//------------------------------------------------------------------------------
+bool vtkF3DQuakeMDLImporter::CanReadFile(vtkResourceStream* stream)
+{
+  if (!stream)
+  {
+    return false;
+  }
+
+  // Read header into buffer
+  std::size_t headerSize = sizeof(vtkInternals::mdl_header_t);
+  stream->Seek(0, vtkResourceStream::SeekDirection::Begin);
+  std::vector<uint8_t> buffer(headerSize);
+  if (stream->Read(buffer.data(), headerSize) != headerSize)
+  {
+    return false;
+  }
+
+  // Check header buffer
+  size_t offset = 0;
+  const vtkInternals::mdl_header_t* header;
+  if (!vtkInternals::ReadAndCheckHeader(buffer, nullptr, offset, header))
+  {
+    return false;
+  }
+  return true;
+}

plugins/native/module/vtkF3DQuakeMDLImporter.h

@@ -70,6 +70,15 @@ class vtkF3DQuakeMDLImporter : public vtkF3DImporter
   vtkGetMacro(SkinIndex, unsigned int);
   ///@}
 
+  /**
+   * Return true if, after a quick check of file header, it looks like the provided stream
+   * can be read. Return false if it is sure it cannot be read as a strean.
+   *
+   * This only checks that the file header contains the right magic ("IPDO" or "IDST") and the
+   * version is 6.
+   */
+  static bool CanReadFile(vtkResourceStream* stream);
+
 protected:
   vtkF3DQuakeMDLImporter();
   ~vtkF3DQuakeMDLImporter() override = default;

testing/data/invalid.mdl

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:28fca28ef02d08bd61931c03999b7a5b822392132867682e88b2db492ee9f087
+size 8