Fixed integer overflow bug in SourceContextReader, adapted TerminaLoggerTest

5 files changed, 106 insertions, 37 deletions

diff --git a/src/core/common/SourceContextReader.cpp b/src/core/common/SourceContextReader.cpp
index 65a6281..d5d379c 100644
--- a/src/core/common/SourceContextReader.cpp
+++ b/src/core/common/SourceContextReader.cpp
@@ -56,8 +56,8 @@ SourceContext SourceContextReader::readContext(CharReader &reader,
 	size_t offs = 0;
 	auto it = std::lower_bound(cache.begin(), cache.end(), start);
 	if (it != cache.begin()) {
-		it--;  // Go to the previous entry
-		offs = *it; // Read the corresponding byte offset
+		it--;        // Go to the previous entry
+		offs = *it;  // Read the corresponding byte offset
 		size_t line = it - cache.begin() + 1;
 		ctx.startLine = line;
 		ctx.endLine = line;
@@ -182,17 +182,23 @@ SourceContext SourceContextReader::readContext(CharReader &reader,
 		}
 
 		// Update the relative position and length, set the "truncated" flags
-		size_t us = static_cast<size_t>(s), ue = static_cast<size_t>(e);
-		ctx.relPos = start - lineBufStart - us;
-		ctx.relLen = std::min(ctx.relLen, ue - us);
+		ctx.relPos = std::max<ssize_t>(0, start - lineBufStart - s);
+		ctx.relLen = std::min<ssize_t>(ctx.relLen, e - s);
 		ctx.truncatedStart = s > ts || lastLineStart < lineBufStart;
 		ctx.truncatedEnd = e < te;
 
 		// Copy the selected area to the output string
-		ctx.text = std::string{&lineBuf[s], ue - us};
+		ctx.text = std::string{&lineBuf[s], static_cast<size_t>(e - s)};
 	}
 
 	return ctx;
 }
+
+SourceContext SourceContextReader::readContext(CharReader &reader,
+                                               const SourceRange &range,
+                                               const std::string &filename)
+{
+	return readContext(reader, range, MAX_MAX_CONTEXT_LENGTH, filename);
+}
 }
 
diff --git a/src/core/common/SourceContextReader.hpp b/src/core/common/SourceContextReader.hpp
index 35e71b3..cd29880 100644
--- a/src/core/common/SourceContextReader.hpp
+++ b/src/core/common/SourceContextReader.hpp
@@ -82,7 +82,25 @@ public:
 	 * @return a SourceContext instance describing the
 	 */
 	SourceContext readContext(CharReader &reader, const SourceRange &range,
-	                          size_t maxContextLength = MAX_MAX_CONTEXT_LENGTH,
+	                          size_t maxContextLength,
+	                          const std::string &filename = "");
+
+	/**
+	 * Returns the context for the char reader and the given SourceRange.
+	 * Returns an invalid source context if either the given range is invalid
+	 * or the byte offset described in the SourceRange cannot be reached because
+	 * the CharReader cannot be seeked back to this position. Does not limit
+	 * the output context.
+	 *
+	 * @param reader is the CharReader instance from which the context should be
+	 * read.
+	 * @param range describes the Range within the source file for which the
+	 * context should be extraced.
+	 * @param filename is the filename that should be stored in the returned
+	 * context.
+	 * @return a SourceContext instance describing the
+	 */
+	SourceContext readContext(CharReader &reader, const SourceRange &range,
 	                          const std::string &filename = "");
 };
 }
diff --git a/src/core/resource/ResourceManager.cpp b/src/core/resource/ResourceManager.cpp
index 059da41..184a16d 100644
--- a/src/core/resource/ResourceManager.cpp
+++ b/src/core/resource/ResourceManager.cpp
@@ -22,6 +22,7 @@
 #include <core/common/Exceptions.hpp>
 #include <core/common/Logger.hpp>
 #include <core/common/Rtti.hpp>
+#include <core/common/SourceContextReader.hpp>
 #include <core/common/Utils.hpp>
 #include <core/model/Node.hpp>
 #include <core/parser/ParserContext.hpp>
@@ -280,5 +281,11 @@ SourceContext ResourceManager::readContext(const SourceLocation &location,
 	}
 	return SourceContext{};
 }
+
+SourceContext ResourceManager::readContext(const SourceLocation &location)
+{
+	return readContext(location, SourceContextReader::MAX_MAX_CONTEXT_LENGTH);
+}
+
 }
 
diff --git a/src/core/resource/ResourceManager.hpp b/src/core/resource/ResourceManager.hpp
index d5381b9..221e2cc 100644
--- a/src/core/resource/ResourceManager.hpp
+++ b/src/core/resource/ResourceManager.hpp
@@ -34,7 +34,6 @@
 
 #include <core/common/Location.hpp>
 #include <core/common/Rtti.hpp>
-#include <core/common/SourceContextReader.hpp>
 #include <core/managed/Managed.hpp>
 
 #include "Resource.hpp"
@@ -230,9 +229,22 @@ public:
 	 * @return a valid SourceContext if a valid SourceLocation was given or an
 	 * invalid SourceContext if the location is invalid.
 	 */
-	SourceContext readContext(
-	    const SourceLocation &location,
-	    size_t maxContextLength = SourceContextReader::MAX_MAX_CONTEXT_LENGTH);
+	SourceContext readContext(const SourceLocation &location,
+	                          size_t maxContextLength);
+	/**
+	 * Creates and returns a SourceContext structure containing information
+	 * about the given SourceLocation (such as line and column number). Throws
+	 * a LoggableException if an irrecoverable error occurs while looking up the
+	 * context (such as a no longer existing resource). Does not limit the
+	 * context length.
+	 *
+	 * @param location is the SourceLocation for which context information
+	 * should be retrieved. This method is used by the Logger class to print
+	 * pretty messages.
+	 * @return a valid SourceContext if a valid SourceLocation was given or an
+	 * invalid SourceContext if the location is invalid.
+	 */
+	SourceContext readContext(const SourceLocation &location);
 };
 }
 
diff --git a/test/core/frontend/TerminalLoggerTest.cpp b/test/core/frontend/TerminalLoggerTest.cpp
index b0e769d..99e8f29 100644
--- a/test/core/frontend/TerminalLoggerTest.cpp
+++ b/test/core/frontend/TerminalLoggerTest.cpp
@@ -35,34 +35,41 @@ struct Pos {
 };
 
 static const std::string testStr =
-    "\\link[domain]{book}\n" // 1
-    "\\link[domain]{meta}\n" // 2
-    "\n" // 3
-    "\\meta{\n" // 4
-    "\t\\title{The Adventures Of Tom Sawyer}\n" // 5
-    "\t\\author{Mark Twain}\n" // 6
-    "}\n" // 7
-    "\n" // 8
-    "\\book{\n" // 9
-    "\n" // 10
-    "\n" // 11
-    "\\chapter\n" // 12
-    "<<TOM!>>\n" // 13
-    "\n" // 14
-    "No answer.\n" // 15
-    "\n" // 16
-    "<<TOM!>>\n" // 17
-    "\n" // 18
-    "No answer.\n" // 19
-    "\n" // 20
-    "<<What's gone with that boy, I wonder? You TOM!>>\n" // 21
-    "}\n"; // 22
+    "\\link[domain]{book}\n"                               // 1
+    "\\link[domain]{meta}\n"                               // 2
+    "\n"                                                   // 3
+    "\\meta{\n"                                            // 4
+    "\t\\title{The Adventures Of Tom Sawyer}\n"            // 5
+    "\t\\author{Mark Twain}\n"                             // 6
+    "}\n"                                                  // 7
+    "\n"                                                   // 8
+    "\\book{\n"                                            // 9
+    "\n"                                                   // 10
+    "\n"                                                   // 11
+    "\\chapter\n"                                          // 12
+    "<<TOM!>>\n"                                           // 13
+    "\n"                                                   // 14
+    "No answer.\n"                                         // 15
+    "\n"                                                   // 16
+    "<<TOM!>>\n"                                           // 17
+    "\n"                                                   // 18
+    "No answer.\n"                                         // 19
+    "\n"                                                   // 20
+    "<<What's gone with that boy, I wonder? You TOM!>>\n"  // 21
+    "}\n";                                                 // 22
 
 static SourceContextReader contextReader{};
 
 static SourceContext contextCallback(const SourceLocation &location)
 {
 	CharReader reader{testStr, 0};
+	return contextReader.readContext(reader, location,
+	                                 "the_adventures_of_tom_sawyer.opd");
+}
+
+static SourceContext truncatedContextCallback(const SourceLocation &location)
+{
+	CharReader reader{testStr, 0};
 	return contextReader.readContext(reader, location, 60,
 	                                 "the_adventures_of_tom_sawyer.opd");
 }
@@ -75,9 +82,18 @@ TEST(TerminalLogger, log)
 
 	logger.debug("This is a test debug message");
 	logger.note("This is a test note");
-	logger.note("This is a test note with point context", SourceLocation{0, 49});
-	logger.note("This is a test note with range context", SourceLocation{0, 49, 55});
-	logger.note("This is a test note with multiline context", SourceLocation{0, 49, 150});
+	logger.note("This is a test note with point context",
+	            SourceLocation{0, 49});
+	logger.note("This is a test note with range context",
+	            SourceLocation{0, 49, 55});
+	logger.note("This is a test note with multiline context",
+	            SourceLocation{0, 49, 150});
+
+	logger.setSourceContextCallback(truncatedContextCallback);
+	logger.note("This is a test note with truncated multiline context",
+	            SourceLocation{0, 49, 150});
+	logger.setSourceContextCallback(contextCallback);
+
 	logger.warning("This is a test warning");
 	logger.error("This is a test error");
 	logger.fatalError("This is a test fatal error!");
@@ -90,6 +106,14 @@ TEST(TerminalLogger, log)
 	catch (const LoggableException &ex) {
 		logger.log(ex);
 	}
+
+	try {
+		throw LoggableException{"An exception with context",
+		                        SourceLocation{0, 41, 46}};
+	}
+	catch (const LoggableException &ex) {
+		logger.log(ex);
+	}
 }
 
 TEST(TerminalLogger, fork)
@@ -101,7 +125,9 @@ TEST(TerminalLogger, fork)
 
 	LoggerFork fork = logger.fork();
 
-	fork.error("This is a test error with context");
+	fork.error("This is a test error without");
+
+	fork.error("This is a test error with context", SourceLocation{0, 6, 12});
 
 	// Print all error messages
 	fork.commit();