Finish up CI implementation

.github/workflows/ci.yml

@@ -43,13 +43,13 @@ jobs:
             cc: clang-16
             cxx: clang++-16
 
-          # macOS with Apple Clang
-          - os: macos-13
+          - os: macos-14
             compiler: clang
             cc: clang
             cxx: clang++
 
-          - os: macos-14
+          # macOS with Apple Clang
+          - os: macos-15
             compiler: clang
             cc: clang
             cxx: clang++
@@ -75,7 +75,7 @@ jobs:
             wget https://apt.llvm.org/llvm.sh
             chmod +x llvm.sh
             sudo ./llvm.sh "$VERSION" || true
-            sudo apt-get install -y ${{ matrix.cxx }}
+            sudo apt-get install -y ${{ matrix.cxx }} clang-tools-${VERSION}
           fi
 
       - name: Install dependencies (macOS)

include/Server.hpp

@@ -1,52 +1,61 @@
 #pragma once
 #include <thread>
+#include <atomic>
 #include "ProtocolStructures.hpp"
 #include "textDocument.hpp"
 #include "Message.hpp"
 #include "iostream"
 
-
-class DocumentHandler {
+class DocumentHandler
+{
       std::map<std::string, textDocument> m_openDocuments;
+
 public:
-      bool openDocument(const std::string& uri, const std::string& document);
-      bool closeDocument(const std::string& uri);
-      bool updateDocument(const std::string& uri, const DidChangeTextDocumentParams& params);
-      bool documentIsOpen(const std::string& uri) const;
+      bool openDocument(const std::string &uri, const std::string &document);
+      bool closeDocument(const std::string &uri);
+      bool updateDocument(const std::string &uri, const DidChangeTextDocumentParams &params);
+      bool documentIsOpen(const std::string &uri) const;
 
       // Returns a reference to the open document if it exists
-      std::optional<std::reference_wrapper<textDocument>> getOpenDocument(const std::string& uri);
+      std::optional<std::reference_wrapper<textDocument>> getOpenDocument(const std::string &uri);
 };
 
-class LSPServer {
+class LSPServer
+{
       std::thread m_listener;
-      bool force_shutdown;
+      std::atomic<bool> force_shutdown;
       bool isOKtoExit;
       bool m_shutdownRequested;
       bool m_initialized;
       ServerCapabilities m_capabilities;
       DocumentHandler m_documentHandler;
 
-      std::istream* m_input_stream;
-      std::ostream* m_output_stream;
+      std::istream *m_input_stream;
+      std::ostream *m_output_stream;
+
 public:
       LSPServer();
       ~LSPServer();
-      int init(const uint64_t& capabilities, std::istream& in = std::cin, std::ostream& out = std::cout);
+      int init(const uint64_t &capabilities, std::istream &in = std::cin, std::ostream &out = std::cout);
       void stop();
       int exit();
-      static void server_main(LSPServer* server);
+      static void server_main(LSPServer *server);
       bool hasCapability(uint64_t capability) const;
-      Response processRequest(const Message& message);
-      void processNotification(const Message& message);
-      void send(const Response& response, bool flush = false);
+      Response processRequest(const Message &message);
+      void processNotification(const Message &message);
+      void send(const Response &response, bool flush = false);
 
       std::optional<hoverResult> hoverCallback(const hoverParams &params);
       definitionResult definitionCallback(const definitionParams &params);
       declarationResult declarationCallback(const declarationParams &params);
 };
 
-
-#define DEFAULT_HOVER_RESULT { {MarkupKind::PlainText, "some response for: " + m_documentHandler.getOpenDocument(params.textDocument.uri).value().get().wordUnderCursor(params.position.line, params.position.character)}, std::nullopt }
-#define DEFAULT_DEFINITION_RESULT { params.textDocument.uri, {{0, 0}, params.position} }
-#define DEFAULT_DECLARATION_RESULT { params.textDocument.uri, {{0, 0}, params.position} }
+#define DEFAULT_HOVER_RESULT {{MarkupKind::PlainText, "some response for: " + m_documentHandler.getOpenDocument(params.textDocument.uri).value().get().wordUnderCursor(params.position.line, params.position.character)}, std::nullopt}
+#define DEFAULT_DEFINITION_RESULT                                \
+      {                                                          \
+            params.textDocument.uri, { {0, 0}, params.position } \
+      }
+#define DEFAULT_DECLARATION_RESULT                               \
+      {                                                          \
+            params.textDocument.uri, { {0, 0}, params.position } \
+      }

src/Message.cpp

@@ -17,12 +17,10 @@ Message::~Message()
 	}
 }
 
-Message::Message(std::istream &buffer)
+Message::Message(std::istream &buffer) : m_buffer(nullptr), m_payloadSize(0)
 {
 	if (buffer.peek() == EOF)
 	{
-		m_buffer = nullptr;
-		m_payloadSize = 0;
 		return;
 	}
 	readMessage(buffer);
@@ -42,28 +40,61 @@ nlohmann::json Message::jsonData() const
 
 int Message::readMessage(std::istream &stream)
 {
+	// Free old buffer if exists (for message reuse)
+	if (m_buffer)
+	{
+		free(m_buffer);
+		m_buffer = nullptr;
+	}
 	m_payloadSize = 0;
 
+	// Check if stream is in good state before reading
+	if (!stream.good() || stream.eof())
+	{
+		return -1; // Signal EOF/error
+	}
+
 	stream.clear();
 	stream.ignore(15);
+
+	// Check for EOF after ignore
+	if (stream.eof() || stream.fail())
+	{
+		return -1;
+	}
+
 	stream >> m_payloadSize;
 
-	// TODO: handle this cleanly
-	if (!(m_payloadSize > 0 && m_payloadSize < INT_MAX))
+	// Check for read failure or EOF
+	// Use reasonable upper limit (10MB) to prevent bad_alloc from malformed/malicious data
+	constexpr size_t MAX_MESSAGE_SIZE = 10 * 1024 * 1024; // 10 MB
+	if (stream.fail() || stream.eof() || m_payloadSize == 0 || m_payloadSize > MAX_MESSAGE_SIZE)
 	{
-		m_buffer = nullptr;
-		m_payloadSize = 0;
-		return 0;
+		return -1;
 	}
 
 	m_buffer = static_cast<char *>(calloc(m_payloadSize + 2, 1));
+	if (!m_buffer)
+	{
+		m_payloadSize = 0;
+		return -1;
+	}
 
 	stream.clear();
 	stream.ignore(4);
 
 	stream.clear();
 	stream.read(m_buffer, m_payloadSize);
 
+	// Check if read was successful
+	if (stream.fail() && !stream.eof())
+	{
+		free(m_buffer);
+		m_buffer = nullptr;
+		m_payloadSize = 0;
+		return -1;
+	}
+
 	m_jsonData = nlohmann::json::parse(m_buffer, nullptr, false);
 
 	return m_payloadSize;

src/Server.cpp

@@ -14,9 +14,9 @@ LSPServer::~LSPServer()
       exit();
 }
 
-int LSPServer::init(const uint64_t& capabilities, std::istream& in, std::ostream& out)
+int LSPServer::init(const uint64_t &capabilities, std::istream &in, std::ostream &out)
 {
-      force_shutdown = false;
+      force_shutdown.store(false);
       isOKtoExit = false;
       m_shutdownRequested = false;
       m_initialized = false;
@@ -32,16 +32,18 @@ int LSPServer::init(const uint64_t& capabilities, std::istream& in, std::ostream
 void LSPServer::stop()
 {
       // TODO: Graceful shutdown actions, interrupt ongoing tasks, etc.
-      force_shutdown = true;
+      force_shutdown.store(true);
       return;
 }
 
 int LSPServer::exit()
 {
       if (m_listener.joinable())
+      {
             // Wait for listener thread to finish
             m_listener.join();
-      return isOKtoExit? 0: 1;
+      }
+      return isOKtoExit ? 0 : 1;
 }
 
 void LSPServer::send(const Response &response, bool flush)
@@ -52,17 +54,22 @@ void LSPServer::send(const Response &response, bool flush)
             m_output_stream->flush();
 }
 
-void LSPServer::server_main(LSPServer* server)
+void LSPServer::server_main(LSPServer *server)
 {
       Message message;
       textDocument document;
 
-      while (!server->force_shutdown)
+      while (!server->force_shutdown.load())
       {
             int readBytes = message.readMessage(*server->m_input_stream);
-            if (readBytes <= 0)
+            if (readBytes < 0)
+            {
+                  // EOF or stream error - exit gracefully
+                  break;
+            }
+            if (readBytes == 0)
             {
-                  // No point in processing invalid message
+                  // Invalid message, but stream is still OK - continue
                   continue;
             }
             Message::log("INBOUND: " + message.get());
@@ -97,7 +104,7 @@ definitionResult LSPServer::definitionCallback(const definitionParams &params)
       return DEFAULT_DEFINITION_RESULT;
 }
 
-declarationResult LSPServer::declarationCallback(const declarationParams & params)
+declarationResult LSPServer::declarationCallback(const declarationParams &params)
 {
       return DEFAULT_DECLARATION_RESULT;
 }
@@ -186,7 +193,7 @@ void LSPServer::processNotification(const Message &message)
             isOKtoExit = (m_shutdownRequested || !m_initialized);
             // EXIT received: stop the server loop now
             stop();
-            break; 
+            break;
       case Message::Method::TEXT_DOCUMENT_DID_OPEN:
       {
             m_documentHandler.openDocument(message.documentURI(), message.params()["textDocument"]["text"]);
@@ -207,42 +214,48 @@ void LSPServer::processNotification(const Message &message)
       }
 }
 
-
-bool DocumentHandler::updateDocument(const std::string& uri, const DidChangeTextDocumentParams& params) {
+bool DocumentHandler::updateDocument(const std::string &uri, const DidChangeTextDocumentParams &params)
+{
 
       auto optionalDocument = getOpenDocument(uri);
       if (!optionalDocument.has_value())
             return false;
 
-      textDocument& document = optionalDocument.value();
+      textDocument &document = optionalDocument.value();
       for (auto &j : params.contentChanges)
       {
             if (j.range.has_value())
             {
-                  const Range& contentChanged = j.range.value();
+                  const Range &contentChanged = j.range.value();
 
                   int startIndex = document.findPos(contentChanged.start.line, contentChanged.start.character);
                   int endIndex = document.findPos(contentChanged.end.line, contentChanged.end.character);
 
                   document.m_content.replace(startIndex, endIndex - startIndex, j.text);
             }
-            else document.m_content = j.text;
+            else
+                  document.m_content = j.text;
       }
 
       return true;
 }
-bool DocumentHandler::openDocument(const std::string& uri, const std::string& document) {
+bool DocumentHandler::openDocument(const std::string &uri, const std::string &document)
+{
       return m_openDocuments.emplace(uri, document).second;
 }
-bool DocumentHandler::closeDocument(const std::string& uri) {
+bool DocumentHandler::closeDocument(const std::string &uri)
+{
       return m_openDocuments.erase(uri) > 0;
 }
-bool DocumentHandler::documentIsOpen(const std::string& uri) const {
+bool DocumentHandler::documentIsOpen(const std::string &uri) const
+{
       return m_openDocuments.count(uri) != 0;
 }
-std::optional<std::reference_wrapper<textDocument>> DocumentHandler::getOpenDocument(const std::string& uri) {
+std::optional<std::reference_wrapper<textDocument>> DocumentHandler::getOpenDocument(const std::string &uri)
+{
       auto it = m_openDocuments.find(uri);
-      if (m_openDocuments.end() == it) {
+      if (m_openDocuments.end() == it)
+      {
             return std::nullopt;
       }
       return std::make_optional<std::reference_wrapper<textDocument>>(it->second);

test/TestClient.hpp

@@ -27,29 +27,35 @@ namespace testutil
             int serverExitCode{0};
       };
 
-      inline std::vector<nlohmann::json> parseAllResponses(const std::string& wire)
+      inline std::vector<nlohmann::json> parseAllResponses(const std::string &wire)
       {
-            static constexpr size_t content_length_header_size = std::string("Content-Length: ").size();
+            static constexpr size_t content_length_header_size = 16; // "Content-Length: ".size()
             std::vector<nlohmann::json> out;
             if (wire.empty())
                   return out;
             size_t pos = 0;
             while (true)
             {
                   auto headerEnd = wire.find("\r\n\r\n", pos);
-                  if (headerEnd == std::string::npos) break;
+                  if (headerEnd == std::string::npos)
+                        break;
 
                   std::string_view header = std::string_view(wire).substr(pos, headerEnd - pos);
                   auto lenPos = header.find("Content-Length: ");
-                  if (lenPos == std::string::npos) break;
+                  if (lenPos == std::string::npos)
+                        break;
 
                   size_t lenStart = lenPos + content_length_header_size;
                   size_t lenEnd = header.find('\r', lenStart);
-                  if (lenEnd == std::string::npos) lenEnd = header.size();
+                  if (lenEnd == std::string::npos)
+                        lenEnd = header.size();
+                  if (lenEnd <= lenStart)
+                        break;
 
                   size_t expected = static_cast<size_t>(std::stoul(std::string(header.substr(lenStart, lenEnd - lenStart))));
                   size_t bodyStart = headerEnd + 4;
-                  if (wire.size() < bodyStart + expected) break;
+                  if (wire.size() < bodyStart + expected)
+                        break;
                   out.emplace_back(nlohmann::json::parse(wire.substr(bodyStart, expected)));
                   pos = bodyStart + expected;
             }
@@ -126,7 +132,8 @@ namespace testutil
             return resp;
       }
 
-      struct BatchResponse {
+      struct BatchResponse
+      {
             std::vector<nlohmann::json> jsonResponses;
             int serverExitCode{0};
       };
@@ -164,7 +171,7 @@ namespace testutil
                   }
                   if (expected_responses > 0 && responses.size() >= expected_responses)
                         break;
-                  
+
                   // std::this_thread::sleep_for(std::chrono::milliseconds(1));
             }
 

test/test_message.cpp

@@ -46,8 +46,8 @@ TEST(Message, parsing)
 
 TEST(Message, parse_method)
 {
-      std::istringstream s("Content-Length: 48\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"textDocument/hover\",\"params\":{\"textDocument\":{\"uri\":\"file:///test.cpp\"},\"position\":{\"line\":10,\"character\":5}},\"id\":2}");
-      std::istringstream s2("Content-Length: 53\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"textDocument/definition\",\"params\":{\"textDocument\":{\"uri\":\"file:///test.cpp\"},\"position\":{\"line\":10,\"character\":5}},\"id\":2}");
+      std::istringstream s("Content-Length: 146\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"textDocument/hover\",\"params\":{\"textDocument\":{\"uri\":\"file:///test.cpp\"},\"position\":{\"line\":10,\"character\":5}},\"id\":2}");
+      std::istringstream s2("Content-Length: 151\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"textDocument/definition\",\"params\":{\"textDocument\":{\"uri\":\"file:///test.cpp\"},\"position\":{\"line\":10,\"character\":5}},\"id\":2}");
 
       Message m(s);
       Message m2(s2);