From bb332cf1e6ada576a5c08ecddfa578f4113f2f1d Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Mon, 25 Nov 2019 15:37:00 +0100
Subject: [PATCH] BUG: ensightReadFile ignores binary string limits (#1511)

ENH: downgrade error on type-mismatch to a warning

- Ensight data files generated by OpenFOAM will start with the name of
  the data type (Eg, "scalar", "vector" etc), but this convention may
  fail for data files that have been generated elsewhere.
---
 .../ensight/read/ensightReadFile.C            |  34 ++++--
 .../readers/ensight/ensightSurfaceReader.C    | 102 ++++++++++--------
 .../ensight/ensightSurfaceReaderTemplates.C   |   9 +-
 3 files changed, 85 insertions(+), 60 deletions(-)

diff --git a/src/fileFormats/ensight/read/ensightReadFile.C b/src/fileFormats/ensight/read/ensightReadFile.C
index c2b0d4c4ca..22c7a2bae8 100644
--- a/src/fileFormats/ensight/read/ensightReadFile.C
+++ b/src/fileFormats/ensight/read/ensightReadFile.C
@@ -2,8 +2,10 @@
   =========                 |
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
-    \\  /    A nd           | Copyright (C) 2016 OpenCFD Ltd.
+    \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
+-------------------------------------------------------------------------------
+    Copyright (C) 2016-2019 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -54,25 +56,37 @@ Foam::Istream& Foam::ensightReadFile::read(string& value)
 {
     if (format() == IOstream::BINARY)
     {
-        char buf[80];
+        auto& iss = stdStream();
+
+        // Binary string is *exactly* 80 characters
+        value.resize(80, '\0');
+        iss.read(&value[0], 80);
 
-        read(reinterpret_cast<char*>(buf), sizeof(buf));
+        if (!iss)
+        {
+            // Truncated - could also exit here, but no real advantage
+            value.erase(iss.gcount());
+        }
 
-        string strBuf(value);
+        // Truncate at the first embedded '\0'
+        auto endp = value.find('\0');
 
-        const size_t iEnd = strBuf.find('\0', 0);
-        if (iEnd == string::npos)
+        if (endp != std::string::npos)
         {
-            value = buf;
+            value.erase(endp);
         }
-        else
+
+        // May have been padded with trailing spaces - remove those
+        endp = value.find_last_not_of(" \t\f\v\n\r");
+
+        if (endp != std::string::npos)
         {
-            value = strBuf.substr(0, iEnd - 1);
+            value.erase(endp + 1);
         }
     }
     else
     {
-        value = "";
+        value.clear();
         while (value.empty() && !eof())
         {
             getLine(value);
diff --git a/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReader.C b/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReader.C
index 05dc8d5bb9..ce95050bfa 100644
--- a/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReader.C
+++ b/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReader.C
@@ -2,8 +2,10 @@
   =========                 |
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
-    \\  /    A nd           | Copyright (C) 2015-2016 OpenCFD Ltd.
+    \\  /    A nd           | www.openfoam.com
      \\/     M anipulation  |
+-------------------------------------------------------------------------------
+    Copyright (C) 2015-2019 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -99,23 +101,23 @@ void Foam::ensightSurfaceReader::readGeometryHeader(ensightReadFile& is) const
 
     // Ensight Geometry File
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Description - 1
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Node info
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Element info
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Part
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Part number
     label ibuffer;
@@ -124,11 +126,11 @@ void Foam::ensightSurfaceReader::readGeometryHeader(ensightReadFile& is) const
 
     // Description - 2
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 
     // Coordinates
     is.read(buffer);
-    DebugInfo<< "buffer: " << buffer << nl;
+    DebugInfo<< "buffer [" << buffer.length() << "] " << buffer << nl;
 }
 
 
@@ -278,29 +280,30 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
 
         streamFormat_ = IOstream::BINARY;
         {
-            istream& is = isBinary.stdStream();
+            istream& iss = isBinary.stdStream();
 
-            char buffer[80];
-            is.read(buffer, 80);
+            // Binary string is *exactly* 80 characters
+            string buf(size_t(80), '\0');
+            iss.read(&buf[0], 80);
 
-            char test[80];
-            label nChar = 0;
-            for (label i = 0; i < 80; ++i)
+            if (!iss)
             {
-                if (buffer[i] == '\0')
-                {
-                    break;
-                }
-                test[i] = buffer[i];
-                nChar++;
+                // Truncated?
+                buf.erase(iss.gcount());
             }
 
-            string testStr(test, nChar);
+            // Truncate at the first embedded '\0'
+            const auto endp = buf.find('\0');
+            if (endp != std::string::npos)
+            {
+                buf.erase(endp);
+            }
 
+            // Contains "C Binary" ?
             if
             (
-                (testStr.find("binary", 0) == string::npos)
-             && (testStr.find("Binary", 0) == string::npos)
+                (buf.find("binary") == std::string::npos)
+             && (buf.find("Binary") == std::string::npos)
             )
             {
                 streamFormat_ = IOstream::ASCII;
@@ -352,8 +355,7 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
         // Read faces - may be a mix of tris, quads and polys
         DynamicList<face> faces(ceil(nPoints/3));
         DynamicList<Tuple2<string, label>> schema(faces.size());
-        string faceType = "";
-        label nFace = 0;
+        string faceType;
         while (is.good()) // (is.peek() != EOF)
         {
             is.read(faceType);
@@ -363,20 +365,22 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
                 break;
             }
 
-            DebugInfo
-                << "faceType: " << faceType << endl;
+            label nFace = 0;
 
             if (faceType == "tria3")
             {
                 is.read(nFace);
 
-                const label np = 3;
-                for (label faceI = 0; faceI < nFace; ++faceI)
+                DebugInfo
+                    << "faceType <" << faceType.c_str() << "> count: "
+                    << nFace << nl;
+
+                face f(3);
+                for (label facei = 0; facei < nFace; ++facei)
                 {
-                    face f(np);
-                    for (label fpI = 0; fpI < np; fpI++)
+                    for (label& fp : f)
                     {
-                        is.read(f[fpI]);
+                        is.read(fp);
                     }
 
                     faces.append(f);
@@ -386,13 +390,16 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
             {
                 is.read(nFace);
 
-                const label np = 4;
-                for (label faceI = 0; faceI < nFace; ++faceI)
+                DebugInfo
+                    << "faceType <" << faceType.c_str() << "> count: "
+                    << nFace << nl;
+
+                face f(4);
+                for (label facei = 0; facei < nFace; ++facei)
                 {
-                    face f(np);
-                    for (label fpI = 0; fpI < np; fpI++)
+                    for (label& fp : f)
                     {
-                        is.read(f[fpI]);
+                        is.read(fp);
                     }
 
                     faces.append(f);
@@ -402,17 +409,21 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
             {
                 is.read(nFace);
 
+                DebugInfo
+                    << "faceType <" << faceType.c_str() << "> count: "
+                    << nFace << nl;
+
                 labelList np(nFace);
-                for (label faceI = 0; faceI < nFace; ++faceI)
+                for (label facei = 0; facei < nFace; ++facei)
                 {
-                    is.read(np[faceI]);
+                    is.read(np[facei]);
                 }
-                for (label faceI = 0; faceI < nFace; ++faceI)
+                for (label facei = 0; facei < nFace; ++facei)
                 {
-                    face f(np[faceI]);
-                    for (label fpI = 0; fpI < f.size(); ++fpI)
+                    face f(np[facei]);
+                    for (label& fp : f)
                     {
-                        is.read(f[fpI]);
+                        is.read(fp);
                     }
 
                     faces.append(f);
@@ -423,11 +434,10 @@ const Foam::meshedSurface& Foam::ensightSurfaceReader::geometry()
                 if (debug)
                 {
                     WarningInFunction
-                        << "Unknown face type: " << faceType
-                        << ".  Aborting read and continuing with current "
+                        << "Unknown face type: <" << faceType.c_str()
+                        << ">. Stopping read and continuing with current "
                         << "elements only" << endl;
                 }
-
                 break;
             }
             schema.append(Tuple2<string, label>(faceType, nFace));
diff --git a/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReaderTemplates.C b/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReaderTemplates.C
index 0275d48231..b56ce103a3 100644
--- a/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReaderTemplates.C
+++ b/src/sampling/sampledSurface/readers/ensight/ensightSurfaceReaderTemplates.C
@@ -111,10 +111,11 @@ Foam::tmp<Foam::Field<Type>> Foam::ensightSurfaceReader::readField
 
     if (primitiveType != pTraits<Type>::typeName)
     {
-        FatalIOErrorInFunction(is)
-            << "Expected " << pTraits<Type>::typeName << "values "
-            << "but found type " << primitiveType
-            << exit(FatalIOError);
+        IOWarningInFunction(is)
+            << "Expected '" << pTraits<Type>::typeName
+            << "' values but found type " << primitiveType << nl
+            << "    This may be okay, but could also indicate an error"
+            << nl << nl;
     }
 
     scalar value;
-- 
GitLab