From bbde03e9d6d837481b32b27a48d8f01ac73d4b98 Mon Sep 17 00:00:00 2001
From: Greg Landrum <greg.landrum@gmail.com>
Date: Wed, 9 May 2012 03:19:30 +0000
Subject: [PATCH] fix and test issue 3524949

---
 Code/GraphMol/FileParsers/SDMolSupplier.cpp   |  4 ++++
 Code/GraphMol/FileParsers/testMolSupplier.cpp | 20 +++++++++++++++++++
 Code/GraphMol/Wrap/rough_test.py              |  2 +-
 .../src-test/org/RDKit/SuppliersTests.java    | 13 ++++++------
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/Code/GraphMol/FileParsers/SDMolSupplier.cpp b/Code/GraphMol/FileParsers/SDMolSupplier.cpp
index e46a7fd10..b2d060550 100644
--- a/Code/GraphMol/FileParsers/SDMolSupplier.cpp
+++ b/Code/GraphMol/FileParsers/SDMolSupplier.cpp
@@ -129,6 +129,10 @@ namespace RDKit {
     
   ROMol *SDMolSupplier::next() {
     PRECONDITION(dp_inStream,"no stream");
+    if(df_end && d_last>=d_len){
+      throw FileParseException("EOF hit.");
+    }
+
     // set the stream to the current position
     dp_inStream->seekg(d_molpos[d_last]);
 
diff --git a/Code/GraphMol/FileParsers/testMolSupplier.cpp b/Code/GraphMol/FileParsers/testMolSupplier.cpp
index 63c0c088e..810e951d8 100644
--- a/Code/GraphMol/FileParsers/testMolSupplier.cpp
+++ b/Code/GraphMol/FileParsers/testMolSupplier.cpp
@@ -52,6 +52,26 @@ int testMolSup() {
     }
     TEST_ASSERT(i==16);
   }
+  {
+    SDMolSupplier sdsup(fname);
+    for(unsigned int i=0;i<16;++i){
+      ROMol *nmol = sdsup.next();
+      if (nmol) {
+        TEST_ASSERT(nmol->hasProp("_Name"));
+        TEST_ASSERT(nmol->hasProp("NCI_AIDS_Antiviral_Screen_Conclusion"));
+        delete nmol;
+      }
+    }
+    // test issue 3524949:
+    TEST_ASSERT(sdsup.atEnd());
+    bool ok=false; 
+    try{
+      ROMol *mol = sdsup.next();
+    } catch (FileParseException &) {
+      ok=true;
+    }
+    TEST_ASSERT(ok);
+  }
   {
     std::ifstream strm(fname.c_str());
     SDMolSupplier sdsup(&strm,false);
diff --git a/Code/GraphMol/Wrap/rough_test.py b/Code/GraphMol/Wrap/rough_test.py
index 7421f49af..71583242b 100755
--- a/Code/GraphMol/Wrap/rough_test.py
+++ b/Code/GraphMol/Wrap/rough_test.py
@@ -783,7 +783,7 @@ class TestCase(unittest.TestCase):
         # test parsed charges on one of the molecules
         for id in chgs192.keys() :
           self.failUnless(mol.GetAtomWithIdx(id).GetFormalCharge() == chgs192[id])
-          
+    self.failUnlessRaises(StopIteration,lambda:sdSup.next())
     sdSup.reset()
     
     ns = [mol.GetProp("_Name") for mol in sdSup]
diff --git a/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/SuppliersTests.java b/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/SuppliersTests.java
index 2ac5246bf..91dd17a54 100644
--- a/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/SuppliersTests.java
+++ b/Code/JavaWrappers/gmwrapper/src-test/org/RDKit/SuppliersTests.java
@@ -91,15 +91,16 @@ public class SuppliersTests extends GraphMolTest {
 		assertEquals("78",m.getProp("_Name"));
 		assertEquals("78",m.getProp("NSC"));
 		assertEquals("6290-84-2",m.getProp("CAS_RN"));
-
-		/* The C++ code does not throw an error when iterating
-		 * past the end, just returns null.
-		 * So test for that */
-		suppl.reset();
+	}
+    @Test(expected=org.RDKit.GenericRDKitException.class)
+	public void test1SDSupplierEOF() {
+		File fileN = new File(baseTestPath, "NCI_aids.10.sdf");
+		SDMolSupplier suppl = new SDMolSupplier(fileN.getPath());
+		ROMol m;
 		for (int i = 0; i < 10; i++)
 			m = suppl.next();
+		assertEquals( true,suppl.atEnd() );
 		m = suppl.next();
-		assertNull("Iterate past end of list returns null", m);
 	}
 
 	@Test