feature: Add wvlet.ai.log for logging utilities

[xerial]

Description

Migrated logging utilities to use the wvlet.ai.log package. Refactored airframe-log as wvlet.ai.log for consistency and maintainability.

Related Issue/Task

Checklist

• This pull request focuses on a single task.
• The change does not contain security credentials

[amazon-q-developer]

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

[amazon-q-developer]

Warning

Description: The flush method doesn't handle potential exceptions that may occur during parent.publish or parent.flush operations. Wrap the publishing and flushing operations in a try-catch block to handle potential exceptions.

Severity: High

[amazon-q-developer]

The fix addresses the issue mentioned in the comment by wrapping the publishing and flushing operations in a try-catch block to handle potential exceptions. Here's a detailed explanation of the changes:

1. The original code:

records.result().map(parent.publish(_))
parent.flush()

2. The fixed code:

try {
  records.result().foreach(parent.publish)
  parent.flush()
} catch {
  case e: Exception =>
    // Handle or log the exception
    e.printStackTrace()
}

The fix makes the following improvements:

1. Exception handling: The try-catch block catches any exceptions that might occur during the publishing or flushing operations. This prevents the application from crashing if an unexpected error occurs.

2. Improved iteration: The map function is replaced with foreach. This is more appropriate because we're not transforming the records, just performing an action on each one.

3. Error logging: In the catch block, we print the stack trace of the exception. In a production environment, you might want to replace this with proper logging or error handling based on your application's requirements.

This fix ensures that any exceptions during the flush operation are caught and handled, improving the robustness of the code. It's worth noting that the specific error handling strategy (printing the stack trace) might need to be adjusted based on the application's logging and error handling policies.

References:

1. Scala documentation on exception handling: https://docs.scala-lang.org/overviews/scala-book/try-catch-finally.html
2. Java logging best practices: https://www.loggly.com/blog/30-best-practices-logging-scale/

Additional example of exception handling in Scala:

def riskyOperation(): Unit = {
  try {
    // Some risky operation
    throw new RuntimeException("Something went wrong")
  } catch {
    case e: RuntimeException => println(s"Caught runtime exception: ${e.getMessage}")
    case _: Throwable => println("Caught unknown error")
  } finally {
    println("This will always be executed")
  }
}

This example shows how to catch specific types of exceptions and provide different handling for each, as well as using a finally block for cleanup operations that should always run.

Suggested change

	records.result().map(parent.publish(_))
	records += record
	}
	try {
	records.result().foreach(parent.publish)
	parent.flush()
	} catch {
	case e: Exception =>
	// Handle or log the exception
	e.printStackTrace()
	}
	override def publish(record: jl.LogRecord): Unit = guard {
	queue.addLast(record)

[amazon-q-developer]

Warning

Description: No error handling for potential buffer overflow in strftime and strcat operations. Add checks for return values of strftime and strcat to ensure buffer overflow doesn't occur.

Severity: High

[amazon-q-developer]

The fix addresses the issue of potential buffer overflow in strftime and strcat operations by adding error handling and checks for the return values of these functions. Here's a detailed explanation of the changes:

1. strftime check:

   • Added a check for the return value of strftime:

     val strftimeResult = strftime(buf, bufSize, pattern, tmPtr)
     if strftimeResult == 0.toUSize then
       return "Error: Buffer overflow in strftime"

   • If strftime returns 0, it indicates a buffer overflow, so we return an error message.

2. strcat checks:

   • Added checks for the return value of strcat:

     if strcat(buf, msBuf) == null then
       return "Error: Buffer overflow in strcat"

   • If strcat returns null, it indicates a buffer overflow, so we return an error message.
   • Similar checks are added for other strcat operations.

3. Increased buffer sizes:

   • Increased the size of msBuf from 3 to 4 bytes to accommodate the null terminator:

     val msBuf: Ptr[Byte] = alloc[Byte](4) // Increased size to 4 for null terminator

   • Increased the size of tzBuf from 5 to 6 bytes for the same reason:

     val tzBuf: Ptr[Byte] = alloc[Byte](6) // Increased size to 6 for potential null terminator

4. Additional strftime check for timezone:

   • Added a check for the return value of strftime when formatting the timezone:

     val tzStrftimeResult = strftime(tzBuf, 6.toUSize, c"%z", tmPtr)
     if tzStrftimeResult == 0.toUSize then
       return "Error: Buffer overflow in timezone strftime"

These changes address the vulnerability by ensuring that buffer overflows are detected and handled gracefully. If any of the string operations fail due to insufficient buffer size, the function will return an error message instead of potentially causing undefined behavior or security issues.

References:

1. strftime documentation: https://en.cppreference.com/w/c/chrono/strftime
2. strcat documentation: https://en.cppreference.com/w/c/string/byte/strcat

Additional example of error handling in C string functions:

#include <stdio.h>
#include <string.h>

#define BUFFER_SIZE 10

int main() {
    char buffer[BUFFER_SIZE];
    const char *source = "This is a long string";
    
    if (strncpy(buffer, source, BUFFER_SIZE - 1) == NULL) {
        printf("Error: strncpy failed\n");
        return 1;
    }
    buffer[BUFFER_SIZE - 1] = '\0'; // Ensure null-termination
    
    printf("Copied string: %s\n", buffer);
    return 0;
}

This example demonstrates how to use strncpy safely with a fixed-size buffer, ensuring null-termination and checking for errors.

Suggested change

	strftime(buf, bufSize, pattern, tmPtr)
	localtime_r(ttPtr, tmPtr)
	val bufSize = 29.toUSize // max size for time strings
	val buf: Ptr[Byte] = alloc[Byte](bufSize)
	val strftimeResult = strftime(buf, bufSize, pattern, tmPtr)
	if strftimeResult == 0.toUSize then
	return "Error: Buffer overflow in strftime"
	val ms = timeMillis % 1000
	val msBuf: Ptr[Byte] = alloc[Byte](4) // Increased size to 4 for null terminator
	sprintf(msBuf, c"%03d", ms)
	if strcat(buf, msBuf) == null then
	return "Error: Buffer overflow in strcat"
	val tzBuf: Ptr[Byte] = alloc[Byte](6) // Increased size to 6 for potential null terminator
	val tzStrftimeResult = strftime(tzBuf, 6.toUSize, c"%z", tmPtr)
	if tzStrftimeResult == 0.toUSize then
	return "Error: Buffer overflow in timezone strftime"
	if strlen(tzBuf) <= 1.toUSize then
	// For UTC-00:00
	if strcat(buf, c"Z") == null then
	return "Error: Buffer overflow in strcat"
	else
	if strcat(buf, tzBuf) == null then
	return "Error: Buffer overflow in strcat"
	fromCString(buf)
	}

[amazon-q-developer]

Warning

Description: The flush() method is empty, which may lead to potential data loss if buffered logs are not written to disk immediately. Implement the flush() method to ensure buffered logs are written to disk. Consider calling fileAppender.getOutputStream().flush().

Severity: High

[amazon-q-developer]

The fix addresses the issue of an empty flush() method, which could potentially lead to data loss if buffered logs are not written to disk immediately. The updated implementation calls fileAppender.getOutputStream().flush() to ensure that any buffered logs are written to disk.

Here's a detailed explanation of the fix:

1. The original code had an empty flush() method:

   override def flush(): Unit = {}

2. The fixed code implements the flush() method:

   override def flush(): Unit = {
     fileAppender.getOutputStream().flush()
   }

This fix ensures that when flush() is called, it will flush the underlying output stream of the fileAppender. This is important because:

1. It forces any buffered data to be written to the disk immediately.
2. It helps prevent data loss in case of unexpected program termination or system crashes.
3. It allows for more real-time logging, which can be crucial for debugging or monitoring purposes.

The fileAppender.getOutputStream().flush() call works because:

1. fileAppender is an instance of RollingFileAppender[String].
2. RollingFileAppender extends OutputStreamAppender, which has a getOutputStream() method.
3. The returned OutputStream has a flush() method that writes any buffered output bytes to the underlying output device.

This implementation assumes that fileAppender is properly initialized and that getOutputStream() returns a valid output stream. If there's any chance that fileAppender or its output stream could be null, additional null checks should be added to prevent potential NullPointerExceptions.

References:

1. Java OutputStream documentation: https://docs.oracle.com/javase/8/docs/api/java/io/OutputStream.html#flush--
2. Logback RollingFileAppender documentation: http://logback.qos.ch/manual/appenders.html#RollingFileAppender

Additional example of flushing in logging systems:

In Java's built-in logging, you might see something like this:

public class CustomHandler extends Handler {
    private final FileOutputStream fos;

    // Constructor and other methods...

    @Override
    public void flush() {
        if (fos != null) {
            try {
                fos.flush();
            } catch (IOException e) {
                // Handle or report the error
            }
        }
    }
}

This example shows how a custom logging handler might implement the flush() method, including error handling which could be considered for the Scala implementation as well.

Suggested change

	override def flush(): Unit = {}
	end fileAppender
	override def flush(): Unit = {
	fileAppender.getOutputStream().flush()
	}
	private def toException(t: Throwable) = new Exception(t.getMessage, t)

[amazon-q-developer]

✅ I finished the code review, and left comments with the issues I found. I will now generate code fix suggestions.

[amazon-q-developer]

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

[amazon-q-developer]

Warning

Description: No error handling for potential buffer overflow in strftime and strcat operations. Add checks for the return values of strftime and strcat to ensure buffer sizes are not exceeded.

Severity: High

[amazon-q-developer]

The fix addresses the issue of potential buffer overflow in the strftime and strcat operations by adding error handling and checks for the return values of these functions.

1. For strftime:

   • We store the result of strftime in a variable strftimeResult.
   • We check if strftimeResult is 0, which indicates an error (usually buffer overflow).
   • If an error occurs, we return an error message.

2. For strcat:

   • We check the return value of strcat, which is null if an error occurs (including buffer overflow).
   • If an error occurs, we return an error message.

These checks are added for all instances of strcat in the code.

This fix helps prevent potential buffer overflows, which could lead to undefined behavior or security vulnerabilities. By checking the return values, we can detect when these operations fail due to insufficient buffer size and handle the errors gracefully.

It's worth noting that this fix assumes that the format function can return a String that represents an error message. In a more robust implementation, you might want to consider using a custom error type or throwing exceptions instead of returning error strings.

References:

1. strftime documentation: https://en.cppreference.com/w/c/chrono/strftime
2. strcat documentation: https://en.cppreference.com/w/c/string/byte/strcat

Additional example of error handling for string functions:

def safeStrCat(dest: Ptr[Byte], src: Ptr[Byte], destSize: USize): Boolean = {
  val remainingSpace = destSize - strlen(dest)
  if strlen(src) < remainingSpace then
    strcat(dest, src)
    true
  else
    false
}

// Usage
if !safeStrCat(buf, msBuf, bufSize) then
  return "Error: Buffer overflow prevented in strcat"

This example demonstrates a safer way to concatenate strings by checking available space before performing the operation.

Suggested change

	strftime(buf, bufSize, pattern, tmPtr)
	localtime_r(ttPtr, tmPtr)
	val bufSize = 29.toUSize // max size for time strings
	val buf: Ptr[Byte] = alloc[Byte](bufSize)
	val strftimeResult = strftime(buf, bufSize, pattern, tmPtr)
	if strftimeResult == 0.toUSize then
	return "Error: Buffer overflow in strftime"
	val ms = timeMillis % 1000
	val msBuf: Ptr[Byte] = alloc[Byte](3)
	sprintf(msBuf, c"%03d", ms)
	if strcat(buf, msBuf) == null then
	return "Error: Buffer overflow in strcat"
	val tzBuf: Ptr[Byte] = alloc[Byte](5)
	strftime(tzBuf, 5.toUSize, c"%z", tmPtr)
	if strlen(tzBuf) <= 1.toUSize then
	// For UTC-00:00
	if strcat(buf, c"Z") == null then
	return "Error: Buffer overflow in strcat"
	else
	if strcat(buf, tzBuf) == null then
	return "Error: Buffer overflow in strcat"
	fromCString(buf)
	}

[amazon-q-developer]

✅ I finished the code review, and left comments with the issues I found. I will now generate code fix suggestions.