Fix an out of bounds access when replacing fragments (#8767)

* Fix an out of bounds access when replacing fragments

* Add segfaulting US patent CDXs

* Return false when we can't find the bond idx for better handling

* Remove useless REQUIRE

* Only sniff when in format AUTO

* Add test for reading wrong format

---------

Co-authored-by: Brian Kelley <bkelley@glysade.com>

External/ChemDraw/chemdraw.cpp

@@ -292,8 +292,15 @@ std::unique_ptr<CDXDocument> streamToCDXDocument(std::istream &inStream,
   } else {
     CDXistream input(inStream);
     const bool doThrow = true;
-    std::unique_ptr<CDXDocument> doc(CDXReadDocFromStorage(input, doThrow));
-    return doc;
+    // if we aren't opened in binary mode on windows, we are going to have a bad time...
+    try {
+      std::unique_ptr<CDXDocument> doc(CDXReadDocFromStorage(input, doThrow));
+      return doc;
+    } catch (const std::exception &ex) {
+      throw FileParseException(
+          std::string(ex.what()) +
+          "\nPlease ensure for CDX data streams, the stream is in binary mode.");
+    }
   }
 }
  
@@ -375,15 +382,22 @@ std::vector<std::unique_ptr<RWMol>> MolsFromChemDrawBlock(
 
 std::vector<std::unique_ptr<RWMol>> MolsFromChemDrawFile(
     const std::string &filename, const ChemDrawParserParams &params) {
-  CDXMLParser parser;
+  ChemDrawParserParams realparams{params};
+  
   std::vector<std::unique_ptr<RWMol>> mols;
 
-  std::fstream chemdrawfile(filename);  // FIX ME CHECK CDX versus CDXML
+  std::fstream chemdrawfile(filename, std::ios::in | std::ios::binary); // Always open in Binary mode
+
   if (!chemdrawfile) {
     throw BadFileException(filename + " does not exist");
     return mols;
   }
-  auto chemdrawmols = molsFromCDXMLDataStream(chemdrawfile, params);
+
+  if (params.format == CDXFormat::AUTO && sniff_format(chemdrawfile) == CDXFormat::CDX)   { // need to reopen in binary mode
+    realparams.format = CDXFormat::CDX;
+  }
+
+  auto chemdrawmols = molsFromCDXMLDataStream(chemdrawfile, realparams);
 
   mols.reserve(chemdrawmols.size());
   for (auto &mol : chemdrawmols) {

External/ChemDraw/test.cpp

@@ -1412,3 +1412,51 @@ TEST_CASE("Geometry") {
     REQUIRE("C1[C@H]2C[C@@H]12" == MolToSmiles(*mols[0]));
   }
 }
+
+TEST_CASE("Test Reading Wrong File Format") {
+    std::string path = std::string(getenv("RDBASE")) + "/Code/GraphMol/test_data/CDXML/";
+  SECTION("CDX as CDXML") { 
+    auto fname = path + "ring-stereo1.cdx";
+    ChemDrawParserParams params;
+    params.format = CDXFormat::CDXML;
+    bool caught = false;
+    try {
+      auto mols = MolsFromChemDrawFile(fname, params);
+    } catch (FileParseException &ex) {
+      caught = true;
+    }
+    REQUIRE(caught);
+  }
+ }
+
+TEST_CASE("github8761") {
+  std::string path =
+      std::string(getenv("RDBASE")) + "/External/ChemDraw/test_data/";
+  SECTION("US12404367-20250902-C00017") {
+    auto fname = path + "US12404367-20250902-C00017.CDX";
+    auto mols = MolsFromChemDrawFile(fname);
+    REQUIRE(mols.size());
+    
+    fname = path + "US12404367-20250902-C00025.CDX";
+    mols = MolsFromChemDrawFile(fname);
+    REQUIRE(mols.size());
+  }    
+
+  SECTION("Failing Patents") {
+    std::filesystem::path patents(path + "patents");
+    for (auto &entry : std::filesystem::directory_iterator(patents)) {
+      ChemDrawParserParams params;
+      params.sanitize = false;
+      auto mols = MolsFromChemDrawFile(entry.path().generic_string(), params);
+      // we just don't want a crash
+
+      std::
+          string block;
+      std::fstream chemdrawfile(entry.path(), std::ios::in | std::ios::binary);
+      std::stringstream buffer;
+      buffer << chemdrawfile.rdbuf();
+      mols = MolsFromChemDrawBlock(buffer.str(), params);
+      // we just don't want a crash
+    }
+  }
+}