Merge pull request #34 from tbirdso/exception-fixes

ENH: Exception fixes and bounds checking

.github/workflows/build-test-package.yml

@@ -110,7 +110,7 @@ jobs:
           "note: prefix with the address-of operator to silence this warning"
           "warning C4722: 'riegeli::internal::UnreachableStream::~UnreachableStream': destructor never returns, potential memory leak"
           "warning C4722: 'riegeli::internal::CheckFailed::~CheckFailed': destructor never returns, potential memory leak"
-          "_deps[/\\].+-src[/\\]"  # disable all warnings from external projects
+          "_deps[/\\\\].+-src[/\\\\]"  # disable all warnings from external projects
           # the following warnings escape the above filter
           "warning: Import validate/validate.proto is unused"
           "warning:.+__builtin___memcpy_chk"

src/itkOMEZarrNGFFImageIO.cxx

@@ -20,6 +20,7 @@
 #include "itkIOCommon.h"
 #include "itkIntTypes.h"
 #include "itkByteSwapper.h"
+#include "itkMacro.h"
 
 #include "tensorstore/context.h"
 #include "tensorstore/open.h"
@@ -208,7 +209,7 @@ writeJson(nlohmann::json json, std::string path, std::string driver)
 
 // JSON file path, e.g. "C:/Dev/ITKIOOMEZarrNGFF/v0.4/cyx.ome.zarr/.zattrs"
 bool
-jsonRead(std::string path, nlohmann::json & result, std::string driver)
+jsonRead(const std::string path, nlohmann::json & result, std::string driver)
 {
   // Reading JSON via TensorStore allows it to be in the cloud
   auto attrs_store = tensorstore::Open<nlohmann::json, 0>(
@@ -306,7 +307,7 @@ OMEZarrNGFFImageIO::ReadArrayMetadata(std::string path, std::string driver)
   }
   else
   {
-    assert(this->GetNumberOfDimensions() == dims.size());
+    itkAssertOrThrowMacro(this->GetNumberOfDimensions() == dims.size(), "Found dimension mismatch in metadata");
   }
 
   for (unsigned d = 0; d < dims.size(); ++d)
@@ -318,15 +319,16 @@ OMEZarrNGFFImageIO::ReadArrayMetadata(std::string path, std::string driver)
 void
 addCoordinateTransformations(OMEZarrNGFFImageIO * io, nlohmann::json ct)
 {
-  assert(ct.is_array());
-  assert(ct.size() >= 1);
+  itkAssertOrThrowMacro(ct.is_array(), "Failed to parse coordinate transforms");
+  itkAssertOrThrowMacro(ct.size() >= 1, "Expected at least one coordinate transform");
+  itkAssertOrThrowMacro(ct[0].at("type") == "scale",
+                        ("Expected first transform to be \"scale\" but found " +
+                         std::string(ct[0].at("type")))); // first transformation must be scale
 
-
-  assert(ct[0].at("type") == "scale"); // first transformation must be scale
   nlohmann::json s = ct[0].at("scale");
-  assert(s.is_array());
+  itkAssertOrThrowMacro(s.is_array(), "Failed to parse scale transform");
   unsigned dim = s.size();
-  assert(dim == io->GetNumberOfDimensions());
+  itkAssertOrThrowMacro(dim == io->GetNumberOfDimensions(), "Found dimension mismatch in scale transform");
 
   for (unsigned d = 0; d < dim; ++d)
   {
@@ -337,11 +339,13 @@ addCoordinateTransformations(OMEZarrNGFFImageIO * io, nlohmann::json ct)
 
   if (ct.size() > 1) // there is also a translation
   {
-    assert(ct[1].at("type") == "translation"); // first transformation must be scale
+    itkAssertOrThrowMacro(ct[1].at("type") == "translation",
+                          ("Expected second transform to be \"translation\" but found " +
+                           std::string(ct[1].at("type")))); // first transformation must be scale
     nlohmann::json tr = ct[1].at("translation");
-    assert(tr.is_array());
+    itkAssertOrThrowMacro(tr.is_array(), "Failed to parse translation transform");
     dim = tr.size();
-    assert(dim == io->GetNumberOfDimensions());
+    itkAssertOrThrowMacro(dim == io->GetNumberOfDimensions(), "Found dimension mismatch in translation transform");
 
     for (unsigned d = 0; d < dim; ++d)
     {
@@ -365,9 +369,14 @@ OMEZarrNGFFImageIO::ReadImageInformation()
   nlohmann::json json;
   std::string    driver = getKVstoreDriver(this->GetFileName());
 
-  bool status = jsonRead(std::string(this->GetFileName()) + "/.zgroup", json, driver);
-  assert(json.at("zarr_format").get<int>() == 2); // only v2 for now
-  status = jsonRead(std::string(this->GetFileName()) + "/.zattrs", json, driver);
+  const std::string zgroupFilePath(std::string(this->GetFileName()) + "/.zgroup");
+  bool              status = jsonRead(zgroupFilePath, json, driver);
+  itkAssertOrThrowMacro(status, ("Failed to read from " + zgroupFilePath));
+  itkAssertOrThrowMacro(json.at("zarr_format").get<int>() == 2, "Only v2 zarr format is supported"); // only v2 for now
+
+  const std::string zattrsFilePath(std::string(this->GetFileName()) + "/.zattrs");
+  status = jsonRead(zattrsFilePath, json, driver);
+  itkAssertOrThrowMacro(status, ("Failed to read from " + zattrsFilePath));
   json = json.at("multiscales")[0]; // multiscales must be present in OME-NGFF
   auto version = json.at("version").get<std::string>();
   if (version == "0.4" || version == "0.3" || version == "0.2" || version == "0.1")
@@ -395,11 +404,11 @@ OMEZarrNGFFImageIO::ReadImageInformation()
     addCoordinateTransformations(this, json.at("coordinateTransformations")); // dataset-level scaling
   }
   json = json.at("datasets");
-  if (this->GetDatasetIndex() > json.size())
+  if (this->GetDatasetIndex() >= json.size())
   {
     itkExceptionMacro(<< "Requested DatasetIndex of " << this->GetDatasetIndex()
-                      << " is greater than number of datasets (" << json.size() << ") which exist in OME-NGFF store '"
-                      << this->GetFileName() << "'");
+                      << " is out of range for the number of datasets (" << json.size()
+                      << ") which exist in OME-NGFF store '" << this->GetFileName() << "'");
   }
 
   json = json[this->GetDatasetIndex()];