From 4c26a4405a5f32677ddf39dd5bfddb94fa3a36dc Mon Sep 17 00:00:00 2001 From: Jeremiah Church Date: Sun, 22 Mar 2026 19:31:45 -0400 Subject: [PATCH] fix: pass pagination limit to log-reading callables to prevent OOM Log endpoints load ALL log files (current + every rotated/compressed copy) into memory before Model::read_all() applies pagination via array_slice(). Even ?limit=1 loads the entire dataset, causing PHP memory exhaustion (512MB) on systems with moderate log volumes. This passes the pagination limit from Model::read_all() through get_internal_objects() to log-reading callables so they stop early. LogFileModelTraits changes: - read_log() accepts optional $limit parameter - When limit > 0, reads newest files first and stops when satisfied - Plain files: reverse-reads from EOF via fseek - O(limit) memory - Compressed files: streams via PHP stream wrappers (compress.zlib://, compress.bzip2://) and popen (xz) with a ring buffer - O(limit) memory for all formats - gather_log_filepaths() uses glob($base.'.*') to avoid matching unrelated files, sorts newest-first by rotation number - Unbounded reads (limit=0) preserve existing behavior exactly Model.inc changes: - get_internal_objects() accepts optional $limit hint - Forwards limit to callables via cached reflection check (backward-compatible with callables that don't accept it) - read_all() passes limit+offset down; skips optimization when reverse=true since that needs the full dataset All 6 log model callables updated to accept and forward limit. 10 new test cases covering bounded reads, edge cases, mixed compression, and unrelated file filtering. Tested on live pfSense (FreeBSD 15.0, 28MB filter.log + 31 rotated bz2 files): memory dropped from 512MB+ (OOM) to 22MB, response time 1ms for ?limit=10. Fixes #806 --- .../usr/local/pkg/RESTAPI/Core/Model.inc | 54 ++- .../ModelTraits/LogFileModelTraits.inc | 327 ++++++++++++++++-- .../usr/local/pkg/RESTAPI/Models/AuthLog.inc | 5 +- .../usr/local/pkg/RESTAPI/Models/DHCPLog.inc | 5 +- .../local/pkg/RESTAPI/Models/FirewallLog.inc | 5 +- .../local/pkg/RESTAPI/Models/OpenVPNLog.inc | 5 +- .../local/pkg/RESTAPI/Models/RESTAPILog.inc | 5 +- .../local/pkg/RESTAPI/Models/SystemLog.inc | 5 +- ...IModelTraitsLogFileModelTraitsTestCase.inc | 261 ++++++++++++++ 9 files changed, 624 insertions(+), 48 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc index 11bce85f2..da9a709fa 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc @@ -922,7 +922,13 @@ class Model { * `config_path` and a `internal_callable` are assigned to this model * @return array The array of internal objects without any additional processing performed. */ - public function get_internal_objects(bool $from_all_parents = false): array { + /** + * @param bool $from_all_parents Whether to obtain objects from all parent Models. + * @param int $limit Pagination hint for internal callables (0 = no limit). When non-zero, callables that accept + * a $limit parameter (e.g. log-backed Models) can use it to avoid loading entire datasets + * into memory. Callables that do not accept a $limit parameter are called without it. + */ + public function get_internal_objects(bool $from_all_parents = false, int $limit = 0): array { global $mock_internal_objects; # Throw an error if both `config_path` and `internal_callable` are set. @@ -947,12 +953,21 @@ class Model { # Obtain the internal objects by calling the `internal_callable` if specified elseif ($this->internal_callable) { $callable = $this->internal_callable; - $internal_objects = $this->$callable(); + + # Forward the pagination limit to callables that accept it. This allows log-backed + # Models to stop reading early instead of loading entire log files into memory. + # Callables that don't accept a $limit parameter continue to work unchanged. + # Reflection results are cached per class+callable to avoid repeated introspection. + if ($limit > 0 && $this->callable_accepts_limit($callable)) { + $internal_objects = $this->$callable(limit: $limit); + } else { + $internal_objects = $this->$callable(); + } } # Otherwise, throw an error. Either a `config_path` or an `internal_callable` is required. else { throw new ServerError( - message: "Model requires a 'config_path' or 'internal_callable' value to be defined before + message: "Model requires a 'config_path' or 'internal_callable' value to be defined before obtaining internal objects.", response_id: 'MODEL_WITH_NO_INTERNAL_METHOD', ); @@ -961,6 +976,31 @@ class Model { return $internal_objects; } + /** + * Checks whether an internal callable method accepts a `limit` parameter. Results are cached + * per class+callable combination to avoid repeated reflection overhead on paginated requests. + * @param string $callable The method name to check. + * @return bool True if the method has a parameter named 'limit'. + */ + private function callable_accepts_limit(string $callable): bool { + static $cache = []; + $key = static::class . '::' . $callable; + + if (!isset($cache[$key])) { + $accepts = false; + $ref = new \ReflectionMethod($this, $callable); + foreach ($ref->getParameters() as $param) { + if ($param->getName() === 'limit') { + $accepts = true; + break; + } + } + $cache[$key] = $accepts; + } + + return $cache[$key]; + } + /** * Obtain this Model object from the internal pfSense configuration by object ID. If the specified ID exists in * config, this Model object will be overwritten with the contents of that object. @@ -1971,8 +2011,12 @@ class Model { return Model::get_model_cache()::fetch_modelset($model_name); } - # Obtain all of this Model's internally stored objects, including those from parent Models if applicable - $internal_objects = $model->get_internal_objects(from_all_parents: true); + # Obtain all of this Model's internally stored objects, including those from parent Models if applicable. + # Pass the maximum number of objects needed so that callables (e.g. log readers) can stop early. + # When $reverse is true, the caller wants the oldest entries, so we cannot pre-limit to the newest - + # the full dataset must be loaded to reverse correctly. + $max_needed = ($limit > 0 && !$reverse) ? ($limit + $offset) : 0; + $internal_objects = $model->get_internal_objects(from_all_parents: true, limit: $max_needed); # For non `many` Models, wrap the internal object in an array so we can loop $internal_objects = $model->many ? $internal_objects : [$internal_objects]; diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/ModelTraits/LogFileModelTraits.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/ModelTraits/LogFileModelTraits.inc index 5cf2435f8..f34ac7b7c 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/ModelTraits/LogFileModelTraits.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/ModelTraits/LogFileModelTraits.inc @@ -81,63 +81,328 @@ trait LogFileModelTraits { } /** - * Gather all log filepaths for a given base log. + * Gather all log filepaths for a given base log, ordered newest-first. The current (unrotated) log file is + * always first, followed by rotated logs in ascending rotation number (0, 1, 2...). + * Only matches the base log itself and files with a rotation suffix (e.g. .0, .1.gz, .2.bz2). * @param string $base_log The base log file path. - * @return array An array of all log file paths for the given base log. + * @return array An array of all log file paths ordered newest-first. */ private function gather_log_filepaths(string $base_log): array { - # Variables $log_filepaths = []; - # Gather all log file paths - foreach (glob($base_log . '*') as $log_filepath) { - $log_filepaths[] = $log_filepath; + # Collect the base log and files with a dot-prefixed suffix (e.g. filter.log.0, filter.log.1.gz). + # Using '.*' instead of '*' prevents matching unrelated files like filter.logrotate. + if (file_exists($base_log)) { + $log_filepaths[] = $base_log; } + foreach (glob($base_log . '.*') as $log_filepath) { + # Skip any glob result that is the base log itself (shouldn't happen with '.*' but be safe) + if ($log_filepath !== $base_log) { + $log_filepaths[] = $log_filepath; + } + } + + # Sort so the base log comes first (newest entries), then rotated by number ascending. + # This ensures the newest log data is always read first regardless of filesystem ordering. + usort($log_filepaths, function (string $a, string $b) use ($base_log): int { + # Base log always comes first (it has the newest entries) + if ($a === $base_log) { + return -1; + } + if ($b === $base_log) { + return 1; + } + + # Extract rotation number from the suffix after the base log name. + # For paths like filter.log.0, filter.log.1.gz, filter.log.10.bz2, we extract the + # numeric portion immediately after the base name dot separator. + # Anchoring to the end with an optional compression extension prevents false matches + # on digits elsewhere in the path (e.g. daemon2.log would not match here since + # we're only looking at the suffix after the base_log prefix). + $suffix_a = substr($a, strlen($base_log)); + $suffix_b = substr($b, strlen($base_log)); + preg_match('/^\.(\d+)/', $suffix_a, $ma); + preg_match('/^\.(\d+)/', $suffix_b, $mb); + $na = isset($ma[1]) ? (int) $ma[1] : PHP_INT_MAX; + $nb = isset($mb[1]) ? (int) $mb[1] : PHP_INT_MAX; + return $na <=> $nb; + }); + + return $log_filepaths; + } + + /** + * Determines the compression type of a log file by normalizing its path (stripping numeric rotation suffixes). + * @param string $filepath The log file path. + * @return string The file extension indicating the log type (e.g. 'log', 'gz', 'bz2', 'xz'). + */ + private function get_log_type(string $filepath): string { + # For uncompressed rotated logs, remove numeric extension to detect the correct type + $normalized = preg_replace('/\.\d+$/', '', $filepath); + return pathinfo($normalized, PATHINFO_EXTENSION); + } + + /** + * Reads the last $limit lines from a plain text file by seeking backwards from EOF. + * Memory usage is O(limit) instead of O(file_size). + * @param string $filepath The path to the log file. + * @param int $limit Maximum number of lines to return from the end of the file. + * @return array The last $limit non-empty lines from the file, in chronological order (oldest first). + */ + private function read_tail_lines(string $filepath, int $limit): array { + if ($limit <= 0) { + return []; + } + + $this->check_file_exists($filepath); + + $fh = fopen($filepath, 'rb'); + if ($fh === false) { + return []; + } + + fseek($fh, 0, SEEK_END); + $pos = ftell($fh); + $buffer = ''; + $lines = []; + $chunk_size = 8192; + + while ($pos > 0 && count($lines) < $limit) { + $read = min($chunk_size, $pos); + $pos -= $read; + fseek($fh, $pos); + $buffer = fread($fh, $read) . $buffer; + + # Split on newlines and collect lines from the end + $parts = explode("\n", $buffer); + # The first element is a partial line (unless we hit BOF) - keep it in buffer + $buffer = array_shift($parts); + + # Collect non-empty lines from the end of the chunk + while (!empty($parts) && count($lines) < $limit) { + $line = array_pop($parts); + $line = rtrim($line, "\r"); + if ($line !== '') { + $lines[] = $line; + } + } + } + + fclose($fh); + + # Don't forget remaining buffer content at the start of the file + $buffer = rtrim($buffer, "\r"); + if ($buffer !== '' && count($lines) < $limit) { + $lines[] = $buffer; + } + + # $lines was collected newest-first; reverse to return in chronological order + return array_reverse($lines); + } + + /** + * Streams a compressed file line-by-line using a file handle, keeping only the last $limit lines + * via a ring buffer. Memory usage is O(limit) instead of O(decompressed_file_size). + * @param resource $fh An already-opened readable file handle (from gzopen, bzopen, or popen). + * @param int $limit Maximum number of lines to keep. + * @return array The last $limit non-empty lines from the stream, in chronological order. + */ + private function stream_tail_lines($fh, int $limit): array { + if ($limit <= 0) { + return []; + } + + $ring = []; + $ring_pos = 0; + $ring_full = false; + $line_buffer = ''; + + # Read in chunks and split into lines manually for compatibility across + # gzopen, bzopen, and popen handles (gzgets works for gz but not others) + while (!feof($fh)) { + $chunk = fread($fh, 8192); + if ($chunk === false || $chunk === '') { + break; + } + $line_buffer .= $chunk; - # Reverse the array so the oldest logs are read first - return array_reverse($log_filepaths); + # Process complete lines from the buffer + while (($newline_pos = strpos($line_buffer, "\n")) !== false) { + $line = rtrim(substr($line_buffer, 0, $newline_pos), "\r"); + $line_buffer = substr($line_buffer, $newline_pos + 1); + + if ($line === '') { + continue; + } + + # Ring buffer: overwrite oldest entry when full + if (!$ring_full && count($ring) < $limit) { + $ring[] = $line; + if (count($ring) === $limit) { + $ring_full = true; + } + } else { + $ring[$ring_pos] = $line; + $ring_pos = ($ring_pos + 1) % $limit; + } + } + } + + # Handle any remaining content without a trailing newline + $line_buffer = rtrim($line_buffer, "\r"); + if ($line_buffer !== '') { + if (!$ring_full && count($ring) < $limit) { + $ring[] = $line_buffer; + } else { + $ring[$ring_pos] = $line_buffer; + $ring_pos = ($ring_pos + 1) % $limit; + } + } + + # Reorder ring buffer to chronological order + if ($ring_full || $ring_pos > 0 && count($ring) === $limit) { + $ring = array_merge( + array_slice($ring, $ring_pos), + array_slice($ring, 0, $ring_pos), + ); + } + + return $ring; + } + + /** + * Reads a compressed log file and returns only the last $limit lines using streaming. + * Uses PHP stream wrappers (compress.zlib://, compress.bzip2://) and popen for xz, + * all of which return standard file handles compatible with fread()/feof(). + * Memory usage is O(limit) instead of O(decompressed_file_size). + * @param string $filepath The path to the compressed log file. + * @param int $limit Maximum number of lines to return. + * @param string $type Compression type: 'gz', 'bz2', or 'xz'. + * @return array The last $limit non-empty lines from the file, in chronological order. + * @throws NotAcceptableError If the compression type is not supported. + */ + private function read_compressed_tail_lines(string $filepath, int $limit, string $type): array { + if ($limit <= 0) { + return []; + } + + $this->check_file_exists($filepath); + + # Use PHP stream wrappers for gz/bz2 so we get standard file handles that work with + # fread()/feof(). This avoids needing separate gzread/bzread functions. + # For xz, use popen since PHP has no built-in xz stream wrapper. + $fh = match ($type) { + 'gz' => fopen('compress.zlib://' . $filepath, 'rb'), + 'bz2' => fopen('compress.bzip2://' . $filepath, 'rb'), + 'xz' => popen('xz -dc ' . escapeshellarg($filepath), 'r'), + default => throw new NotAcceptableError( + message: "Unsupported compression type '$type' for bounded log reading.", + response_id: 'LOG_FILE_TRAITS_UNSUPPORTED_COMPRESSION_TYPE', + ), + }; + + if ($fh === false) { + return []; + } + + $lines = $this->stream_tail_lines($fh, $limit); + + # Close the handle with the appropriate function + match ($type) { + 'xz' => pclose($fh), + default => fclose($fh), + }; + + return $lines; } /** - * Reads all available logs for a given base log file path and returns the contents as an array of lines. Warning: - * This will include rotated logs, including compressed logs which may have a performance impact. + * Reads all available logs for a given base log file path and returns the contents as an array of lines. + * This will include rotated logs, including compressed logs. + * + * When $limit > 0, reads newest log files first and stops as soon as enough lines are collected, avoiding + * loading the entire log history into memory. For the common case of small limits (e.g. ?limit=50), this + * typically only reads the current log file and skips all compressed rotated copies entirely. + * + * When $limit is 0 (default), all log files are read for full backward compatibility. + * * @note zstd compressed logs are not supported. * @param string $base_log The base log file path. + * @param int $limit Maximum total lines to return. 0 means unlimited (read everything). * @return array An array of all log file contents for the given base log. */ - public function read_log(string $base_log): array { - # Variables - $log_contents = []; - - # Ensurethe base log file exists + public function read_log(string $base_log, int $limit = 0): array { + # Ensure the base log file exists $this->check_file_exists($base_log); - # Gather all log file paths + # Gather all log file paths (ordered newest-first for bounded reads) $log_filepaths = $this->gather_log_filepaths($base_log); - # Read each log file + # Unbounded read: preserve existing behavior exactly (read oldest-first, return all) + if ($limit <= 0) { + $log_contents = []; + + # Read oldest-first (reverse of our newest-first ordering) + foreach (array_reverse($log_filepaths) as $log_filepath) { + $type = $this->get_log_type($log_filepath); + + # Determine the log file type and add the contents to the log contents array + $log_contents = match ($type) { + 'bz2' => array_merge($log_contents, $this->read_bzip2_log($log_filepath)), + 'gz' => array_merge($log_contents, $this->read_gzip_log($log_filepath)), + 'xz' => array_merge($log_contents, $this->read_xz_log($log_filepath)), + 'log' => array_merge($log_contents, $this->read_uncompressed_log($log_filepath)), + default => throw new NotAcceptableError( + message: "The log file at $log_filepath has an unsupported file extension.", + response_id: 'LOG_FILE_TRAITS_UNSUPPORTED_LOG_FILE_EXTENSION', + ), + }; + } + + # Clear out empty array elements and reindex the array + $log_contents = array_filter($log_contents); + $log_contents = array_values($log_contents); + + # Map the log contents so each entry is an object with a 'text' property + return array_map(fn($line) => ['text' => $line], $log_contents); + } + + # Bounded read: collect lines from newest files first, stop when we have enough. + # This is the key optimization - for small limits, we typically only read the current + # log file and skip all compressed rotated copies entirely. + $collected = []; + foreach ($log_filepaths as $log_filepath) { - # For uncompressed rotated logs, remove numeric extension to use the correct method to read it below - $log_filepath_normalized = preg_replace('/\.\d+$/', '', $log_filepath); - - # Determine the log file type and add the contents of the log file to the log contents array - $log_contents = match (pathinfo($log_filepath_normalized, PATHINFO_EXTENSION)) { - 'bz2' => array_merge($log_contents, $this->read_bzip2_log($log_filepath)), - 'gz' => array_merge($log_contents, $this->read_gzip_log($log_filepath)), - 'xz' => array_merge($log_contents, $this->read_xz_log($log_filepath)), - 'log' => array_merge($log_contents, $this->read_uncompressed_log($log_filepath)), + $remaining = $limit - count($collected); + if ($remaining <= 0) { + break; + } + + $type = $this->get_log_type($log_filepath); + + # Use memory-efficient tail readers for bounded reads + $lines = match ($type) { + 'log' => $this->read_tail_lines($log_filepath, $remaining), + 'gz', 'bz2', 'xz' => $this->read_compressed_tail_lines($log_filepath, $remaining, $type), default => throw new NotAcceptableError( message: "The log file at $log_filepath has an unsupported file extension.", response_id: 'LOG_FILE_TRAITS_UNSUPPORTED_LOG_FILE_EXTENSION', ), }; + + if (!empty($lines)) { + # Prepend: lines from older (higher rotation number) files go before newer ones + $collected = array_merge($lines, $collected); + } } - # Clear out empty array elements and reindex the array - $log_contents = array_filter($log_contents); - $log_contents = array_values($log_contents); + # Take exactly $limit entries from the end (the newest ones) + if (count($collected) > $limit) { + $collected = array_slice($collected, -$limit); + } - # Map the log contents so each entry is an object with a 'text' property - return array_map(fn($line) => ['text' => $line], $log_contents); + # Filter empties, reindex, and wrap in ['text' => ...] format + $collected = array_values(array_filter($collected)); + return array_map(fn($line) => ['text' => $line], $collected); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/AuthLog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/AuthLog.inc index b178baa4a..1eb57f3dd 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/AuthLog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/AuthLog.inc @@ -31,9 +31,10 @@ class AuthLog extends Model { /** * Obtains the auth log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The auth log as an array of objects. */ - protected function get_auth_log(): array { - return $this->read_log($this->log_file); + protected function get_auth_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/DHCPLog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/DHCPLog.inc index 1e3483345..36351b0fe 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/DHCPLog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/DHCPLog.inc @@ -31,9 +31,10 @@ class DHCPLog extends Model { /** * Obtains the DHCP log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The DHCP log as an array of objects. */ - protected function get_dhcp_log(): array { - return $this->read_log($this->log_file); + protected function get_dhcp_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/FirewallLog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/FirewallLog.inc index 2aeece43a..85da93c5e 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/FirewallLog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/FirewallLog.inc @@ -31,9 +31,10 @@ class FirewallLog extends Model { /** * Obtains the firewall log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The firewall log as an array of objects. */ - protected function get_firewall_log(): array { - return $this->read_log($this->log_file); + protected function get_firewall_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNLog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNLog.inc index e98d3599b..08f9b349d 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNLog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNLog.inc @@ -31,9 +31,10 @@ class OpenVPNLog extends Model { /** * Obtains the openvpn log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The openvpn log as an array of objects. */ - protected function get_openvpn_log(): array { - return $this->read_log($this->log_file); + protected function get_openvpn_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPILog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPILog.inc index bbd3844ee..a4705b61c 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPILog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPILog.inc @@ -31,9 +31,10 @@ class RESTAPILog extends Model { /** * Obtains the REST API log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The REST API log as an array of objects. */ - protected function get_restapi_log(): array { - return $this->read_log($this->log_file); + protected function get_restapi_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemLog.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemLog.inc index b9d639a3f..73c9f6d66 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemLog.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemLog.inc @@ -31,9 +31,10 @@ class SystemLog extends Model { /** * Obtains the system log as an array. This method is the internal callable for this Model. + * @param int $limit Maximum entries to return (0 = all). Passed through from pagination. * @return array The system log as an array of objects. */ - protected function get_system_log(): array { - return $this->read_log($this->log_file); + protected function get_system_log(int $limit = 0): array { + return $this->read_log($this->log_file, $limit); } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc index 8c84fd8a4..6ad30d2cc 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelTraitsLogFileModelTraitsTestCase.inc @@ -7,6 +7,23 @@ use RESTAPI\Core\TestCase; use RESTAPI\Models\SystemLog; class APIModelTraitsLogFileModelTraitsTestCase extends TestCase { + /** + * Clean up temporary test log files before and after each test to prevent cross-contamination. + */ + private function cleanup_test_files(): void { + foreach (glob('/tmp/test_*.log*') as $file) { + @unlink($file); + } + } + + public function setup(): void { + $this->cleanup_test_files(); + } + + public function teardown(): void { + $this->cleanup_test_files(); + } + /** * Checks that the LogFileModelTraits::check_file_exists() method throws a NotFoundError when the file does not exist. */ @@ -163,4 +180,248 @@ class APIModelTraitsLogFileModelTraitsTestCase extends TestCase { $log, ); } + + /** + * Checks that read_log() with a limit returns only the newest entries from uncompressed logs. + */ + public function test_read_log_bounded_uncompressed(): void { + $model = new SystemLog(); + + # Create mock log files + file_put_contents('/tmp/test_bounded.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_bounded.log.0', "Line 4\nLine 5\nLine 6\n"); + file_put_contents('/tmp/test_bounded.log.1', "Line 1\nLine 2\nLine 3\n"); + + # Read with limit=4 should return the 4 newest lines + $log = $model->read_log('/tmp/test_bounded.log', limit: 4); + + $this->assert_equals( + [ + ['text' => 'Line 6'], + ['text' => 'Line 7'], + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that read_log() with a limit smaller than the current log file only reads the current file. + */ + public function test_read_log_bounded_current_file_only(): void { + $model = new SystemLog(); + + # Create mock log files + file_put_contents('/tmp/test_current.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_current.log.0', "Line 4\nLine 5\nLine 6\n"); + file_put_contents('/tmp/test_current.log.1', "Line 1\nLine 2\nLine 3\n"); + + # Read with limit=2 should return only the 2 newest lines from the current log + $log = $model->read_log('/tmp/test_current.log', limit: 2); + + $this->assert_equals( + [ + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that read_log() with a limit works correctly with gzip compressed rotated logs. + */ + public function test_read_log_bounded_gzip(): void { + $model = new SystemLog(); + + # Create mock log files with gzip compressed rotated logs + file_put_contents('/tmp/test_bounded_gz.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_bounded_gz.log.0.gz', gzencode("Line 4\nLine 5\nLine 6\n")); + file_put_contents('/tmp/test_bounded_gz.log.1.gz', gzencode("Line 1\nLine 2\nLine 3\n")); + + # Read with limit=5 should return the 5 newest lines across current and first rotated file + $log = $model->read_log('/tmp/test_bounded_gz.log', limit: 5); + + $this->assert_equals( + [ + ['text' => 'Line 5'], + ['text' => 'Line 6'], + ['text' => 'Line 7'], + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that read_log() with a limit works correctly with bzip2 compressed rotated logs (streaming). + */ + public function test_read_log_bounded_bzip2(): void { + $model = new SystemLog(); + + # Create mock log files with bzip2 compressed rotated logs + file_put_contents('/tmp/test_bounded_bz2.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_bounded_bz2.log.0.bz2', bzcompress("Line 4\nLine 5\nLine 6\n")); + file_put_contents('/tmp/test_bounded_bz2.log.1.bz2', bzcompress("Line 1\nLine 2\nLine 3\n")); + + # Read with limit=5 should return the 5 newest lines + $log = $model->read_log('/tmp/test_bounded_bz2.log', limit: 5); + + $this->assert_equals( + [ + ['text' => 'Line 5'], + ['text' => 'Line 6'], + ['text' => 'Line 7'], + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that read_log() with limit=0 returns all entries (backward compatibility). + */ + public function test_read_log_unbounded_returns_all(): void { + $model = new SystemLog(); + + # Create mock log files + file_put_contents('/tmp/test_unbounded.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_unbounded.log.0', "Line 4\nLine 5\nLine 6\n"); + + # Read with limit=0 (default) should return all entries + $log = $model->read_log('/tmp/test_unbounded.log'); + + $this->assert_equals( + [ + ['text' => 'Line 4'], + ['text' => 'Line 5'], + ['text' => 'Line 6'], + ['text' => 'Line 7'], + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that read_log() with a limit larger than total entries returns all entries. + */ + public function test_read_log_bounded_exceeds_total(): void { + $model = new SystemLog(); + + # Create mock log files with only 6 total lines + file_put_contents('/tmp/test_exceed.log', "Line 4\nLine 5\nLine 6\n"); + file_put_contents('/tmp/test_exceed.log.0', "Line 1\nLine 2\nLine 3\n"); + + # Read with limit=100 should return all 6 entries (not error) + $log = $model->read_log('/tmp/test_exceed.log', limit: 100); + + $this->assert_equals( + [ + ['text' => 'Line 1'], + ['text' => 'Line 2'], + ['text' => 'Line 3'], + ['text' => 'Line 4'], + ['text' => 'Line 5'], + ['text' => 'Line 6'], + ], + $log, + ); + } + + /** + * Checks that read_log() with limit=1 returns only the single newest entry. + */ + public function test_read_log_bounded_single_line(): void { + $model = new SystemLog(); + + file_put_contents('/tmp/test_single.log', "Line 4\nLine 5\nLine 6\n"); + file_put_contents('/tmp/test_single.log.0', "Line 1\nLine 2\nLine 3\n"); + + # Read with limit=1 should return only the newest line + $log = $model->read_log('/tmp/test_single.log', limit: 1); + + $this->assert_equals( + [ + ['text' => 'Line 6'], + ], + $log, + ); + } + + /** + * Checks that read_log() handles an empty current log file gracefully. + */ + public function test_read_log_bounded_empty_current(): void { + $model = new SystemLog(); + + # Empty current log, rotated file has data + file_put_contents('/tmp/test_empty.log', ''); + file_put_contents('/tmp/test_empty.log.0', "Line 1\nLine 2\nLine 3\n"); + + # Bounded read should fall through to the rotated file + $log = $model->read_log('/tmp/test_empty.log', limit: 2); + + $this->assert_equals( + [ + ['text' => 'Line 2'], + ['text' => 'Line 3'], + ], + $log, + ); + } + + /** + * Checks that read_log() handles mixed compression types in rotated logs. + */ + public function test_read_log_bounded_mixed_compression(): void { + $model = new SystemLog(); + + file_put_contents('/tmp/test_mixed.log', "Line 7\nLine 8\nLine 9\n"); + file_put_contents('/tmp/test_mixed.log.0', "Line 4\nLine 5\nLine 6\n"); + file_put_contents('/tmp/test_mixed.log.1.gz', gzencode("Line 1\nLine 2\nLine 3\n")); + + # Read with limit=7 should span all three files with mixed formats + $log = $model->read_log('/tmp/test_mixed.log', limit: 7); + + $this->assert_equals( + [ + ['text' => 'Line 3'], + ['text' => 'Line 4'], + ['text' => 'Line 5'], + ['text' => 'Line 6'], + ['text' => 'Line 7'], + ['text' => 'Line 8'], + ['text' => 'Line 9'], + ], + $log, + ); + } + + /** + * Checks that gather_log_filepaths() does not match unrelated files sharing a prefix. + */ + public function test_read_log_ignores_unrelated_files(): void { + $model = new SystemLog(); + + file_put_contents('/tmp/test_prefix.log', "Line 1\nLine 2\n"); + # These files share the prefix but are NOT rotated logs - they should be ignored + file_put_contents('/tmp/test_prefix.logrotate', "unrelated config data\n"); + file_put_contents('/tmp/test_prefix.log_backup', "backup data\n"); + + # Unbounded read should only return lines from the actual log file + $log = $model->read_log('/tmp/test_prefix.log'); + + $this->assert_equals( + [ + ['text' => 'Line 1'], + ['text' => 'Line 2'], + ], + $log, + ); + } }