Skip to content

Fix: Prevent LogMonitor crash when log line exceeds 4000 characters #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CharityKathure merged 1 commit into from
Jul 1, 2025
Merged

Fix: Prevent LogMonitor crash when log line exceeds 4000 characters #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion LogMonitor/LogMonitorTests/ConfigFileParserTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,7 +1592,7 @@ namespace LogMonitorTests
}";

std::wofstream wof;
wof.imbue(std::locale(std::locale::empty(), new std::codecvt_utf8<wchar_t, 0x10ffff, std::generate_header>));
wof.imbue(std::locale(std::locale(), new std::codecvt_utf8<wchar_t, 0x10ffff, std::generate_header>));
wof.open(fullFileName);
wof << configFileStr;
wof.close();
Expand Down
173 changes: 64 additions & 109 deletions LogMonitor/src/LogMonitor/ProcessMonitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,48 +237,41 @@
/// details from the JSON schema.
/// Returns the number of bytes written to the buffer.
///
size_t FormatProcessLog(char* chBuf) {
std::string FormatProcessLog(const std::string& line) {
if (Utility::CompareWStrings(loggingformat, L"Custom")) {
return FormatCustomLog(chBuf);
return FormatCustomLog(line);
} else {
return FormatStandardLog(chBuf);
return FormatStandardLog(line);
}
}

///
/// Helper function to format the custom log.
///
size_t FormatCustomLog(char* chBuf) {
std::string FormatCustomLog(const std::string& inputLine) {
ProcessLogEntry logEntry;
SYSTEMTIME st;
GetSystemTime(&st);

char chBufCpy[BUFSIZE] = "";
size_t chBufLen = BufferCopyAndSanitize(chBufCpy, chBuf);

logEntry.source = L"Process";
logEntry.currentTime = Utility::SystemTimeToString(st).c_str();

std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t> fromBytesconverter;
logEntry.message = fromBytesconverter.from_bytes(chBufCpy);
std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>, wchar_t> fromBytesConverter;
logEntry.message = fromBytesConverter.from_bytes(inputLine);

std::wstring_convert<std::codecvt_utf8<wchar_t>> toBytesconverter;
std::wstring formattedLog = Utility::FormatEventLineLog(processCustomLogFormat, &logEntry, logEntry.source);
std::string convertedLog = toBytesconverter.to_bytes(formattedLog);
std::string str = convertedLog + "\n";

const char* logLine = str.c_str();
size_t logLineLen = strlen(logLine);
std::wstring_convert<std::codecvt_utf8<wchar_t>> toBytesConverter;
std::string logLine = toBytesConverter.to_bytes(formattedLog) + "\n";

return BufferCopy(chBuf, const_cast<char*>(logLine), 0, logLineLen);
return logLine;
}

///
/// Helper function to format the standard log (JSON or XML).
///
size_t FormatStandardLog(char* chBuf) {
const char* prefix;
const char* suffix;
std::string FormatStandardLog(const std::string& inputLine) {
std::string prefix, suffix;

if (Utility::CompareWStrings(loggingformat, L"XML")) {
prefix = "<Log><Source>Process</Source><LogEntry><Logline>";
Expand All @@ -287,34 +280,20 @@
prefix = "{\"Source\":\"Process\",\"LogEntry\":{\"Logline\":\"";
suffix = "\"},\"SchemaVersion\":\"1.0.0\"}\n";
}
char chBufCpy[BUFSIZE] = "";

//
// copy valid (>0 ASCII values) bytes from chBuf to chBufCpy
//
size_t chBufLen = BufferCopyAndSanitize(chBufCpy, chBuf);
size_t prefixLen = strlen(prefix);
size_t suffixLen = strlen(suffix);
size_t index = BufferCopy(chBuf, const_cast<char*>(prefix), 0, prefixLen);

index = BufferCopy(chBuf, chBufCpy, index);

// truncate, in the unlikely event of a long logline > |BUFSIZE-85|
// leave at least 36 slots for JSON or 21 slots for XML
// reset the start index
if ((index + suffixLen) > BUFSIZE - 5) {
index = BUFSIZE - 5 - suffixLen;
if (Utility::CompareWStrings(loggingformat, L"XML"))
{
suffix = "...\</Logline></LogEntry></Log>\n";
} else {
suffix = "...\"},\"SchemaVersion\":\"1.0.0\"}\n";
}
// Sanitize the log line
std::string sanitized;
sanitized.reserve(inputLine.size());
for (char c : inputLine) {
sanitized += (c > 0) ? c : '?';
Copy link
Contributor

@profnandaa profnandaa Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we didn't support multi-byte (wide) characters then when we were using char* , now that we moved to std::string, could we consider supporting as the next step, perhaps not on this PR. Might come with some perf hits...

Copy link
Contributor Author

@CharityKathure CharityKathure Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added this to our backlog. Thanks.

}

return BufferCopy(chBuf, const_cast<char*>(suffix), index, index + suffixLen);
std::string logLine = prefix + sanitized + suffix;

return logLine;
}


///
/// Helper function to clear the stdout buffer
/// return number of bytes cleared
Expand All @@ -341,85 +320,61 @@
///
DWORD ReadFromPipe(LPVOID Param)
{
DWORD dwRead, dwWritten;
char chBuf[BUFSIZE] = "";
char chBufOut[BUFSIZE] = "";
char chBufRem[BUFSIZE] = ""; // for remaining characters
bool bSuccess = false;
DWORD dwWritten;
char chBuf[BUFSIZE + 1] = { 0 };
std::string partialLine;
HANDLE hParentStdOut = GetStdHandle(STD_OUTPUT_HANDLE);

UNREFERENCED_PARAMETER(Param);

for (;;)
{
// clear buffer ready for read
ClearBuffer(chBuf);
// move valid chars from remainder buffer to chBuf
// then ReadFile starts from the end position (chBuf + cnt)
char* ptrRem = chBufRem;
size_t cnt = 0;
while (*ptrRem > 0 && cnt < BUFSIZE) {
chBuf[cnt++] = *ptrRem;
ptrRem++;
}
DWORD bytesRead = 0;
BOOL bSuccess = ReadFile(g_hChildStd_OUT_Rd, chBuf, BUFSIZE, &bytesRead, NULL);

if (!bSuccess || bytesRead == 0)
break;

bSuccess = ReadFile(g_hChildStd_OUT_Rd, chBuf + cnt, BUFSIZE - cnt, &dwRead, NULL);
// clear remainder buffer for the next read
ClearBuffer(chBufRem);
chBuf[bytesRead] = '\0'; // Null-terminate buffer
partialLine.append(chBuf);

if (!bSuccess || dwRead == 0)
size_t start = 0;
size_t newlinePos;
while ((newlinePos = partialLine.find_first_of("\r\n", start)) != std::string::npos)
{
break;
std::string line = partialLine.substr(start, newlinePos - start);

std::string formatted = FormatProcessLog(line);
logWriter.WriteLog(
hParentStdOut,
formatted.c_str(),
static_cast<DWORD>(formatted.size()),
&dwWritten,
NULL);

// Skip over newline chars
start = newlinePos + 1;
while (start < partialLine.size() &&
(partialLine[start] == '\r' || partialLine[start] == '\n'))
++start;
}

// write out a line at a time
char* ptr = chBuf;
size_t outSz = 0;
size_t count = 0;
size_t lastNewline = 0;
ClearBuffer(chBufOut);
while (*ptr != '\0' && count < BUFSIZE) {
// copy over to chBufOut till \r\n
// the remaining will be reserved to be completed
// by the next read.
if (*ptr == '\r' || *ptr == '\n') {
if (outSz > 0) {
// print out and reset chBufOut and outSz
size_t sz = FormatProcessLog(chBufOut);
DWORD dwRead = static_cast<DWORD>(sz);

bSuccess = logWriter.WriteLog(
hParentStdOut,
chBufOut,
dwRead,
&dwWritten,
NULL);
// reset
outSz = 0;
ClearBuffer(chBufOut);
lastNewline = count;
}
}
else {
// no support for multibyte characters for now
// TODO(nandaa): https://github.com/microsoft/windows-container-tools/issues/121
if (*ptr < 0) {
chBufOut[outSz++] = '?';
}
else chBufOut[outSz++] = *ptr;
}
ptr++;
count++;
}
// Keep unprocessed remainder
if (start < partialLine.size())
partialLine = partialLine.substr(start);
else
partialLine.clear();
}

// move remaining characters to chBufRem
ptrRem = chBuf;
ptrRem += lastNewline;
count = 0;
while (*ptrRem > 0 && count < BUFSIZE) {
chBufRem[count++] = *ptrRem;
ptrRem++;
}
// Write remaining partial line
if (!partialLine.empty())
{
std::string formatted = FormatProcessLog(partialLine);

Check warning on line 371 in LogMonitor/src/LogMonitor/ProcessMonitor.cpp

View workflow job for this annotation

GitHub Actions / Lint C++ 

[cpplint] reported by reviewdog 🐶
 Add #include <string> for string  [build/include_what_you_use] [4]

Raw Output:
LogMonitor/src/LogMonitor/ProcessMonitor.cpp:371:  Add #include <string> for string  [build/include_what_you_use] [4]
logWriter.WriteLog(
hParentStdOut,
formatted.c_str(),
static_cast<DWORD>(formatted.size()),
&dwWritten,
NULL);
}

return ERROR_SUCCESS;
Expand Down
6 changes: 3 additions & 3 deletions LogMonitor/src/LogMonitor/ProcessMonitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@

static size_t ClearBuffer(char* chBuf);

size_t FormatProcessLog(char* chBuf);
std::string FormatProcessLog(const std::string& line);

size_t FormatCustomLog(char* chBuf);
std::string FormatCustomLog(const std::string& line);

size_t FormatStandardLog(char* chBuf);
std::string FormatStandardLog(const std::string& line);

Check warning on line 26 in LogMonitor/src/LogMonitor/ProcessMonitor.h

View workflow job for this annotation

GitHub Actions / Lint C++ 

[cpplint] reported by reviewdog 🐶
 Add #include <string> for string  [build/include_what_you_use] [4]

Raw Output:
LogMonitor/src/LogMonitor/ProcessMonitor.h:26:  Add #include <string> for string  [build/include_what_you_use] [4]

static size_t BufferCopy(char* dst, char* src, size_t start, size_t end);

Expand Down