From 75de5554e309a1ca5d20af059ab9953bd4695241 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Fri, 27 Jun 2025 16:54:47 -0600 Subject: [PATCH 1/5] fix: handle inconsistent delimiter for SystemStatus `cpu_load_average` #716 This commit fixes a bug where pfSense's get_load_average() function returns a load average string that changes based on the system's language setting. Previously, the numbers in the load average string might be separated by just a space, or by a comma and a space. This inconsistency caused an error when the API tried to convert that string into a usable array of values. Now, we're ensuring the load average string is always delimited by a single space, no matter what language pfSense is using. This prevents the logic error and ensures the API can correctly convert the load average to its intended array representation. --- .../local/pkg/RESTAPI/Models/SystemStatus.inc | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc index fb0d240ae..de988ac03 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc @@ -116,7 +116,7 @@ class SystemStatus extends Model { allow_null: true, read_only: true, many: true, - delimiter: ', ', + delimiter: ' ', help_text: 'The CPU load averages. The first value represents the load average for the last minute, the ' . 'second value represents the load average for the last 5 minutes, and the third value represents the ' . 'load average for the last 15 minutes.', @@ -181,7 +181,7 @@ class SystemStatus extends Model { 'mds_mitigation' => get_single_sysctl('hw.mds_disable_state'), 'temp_c' => $this->get_system_temp(), 'temp_f' => $this->get_system_temp(as_fahrenheit: true), - 'cpu_load_avg' => get_load_average(), + 'cpu_load_avg' => $this->get_cpu_load_average(), 'cpu_count' => get_single_sysctl('hw.ncpu'), 'cpu_usage' => $this->get_cpu_usage(), 'mbuf_usage' => $this->get_mbuf_usage(), @@ -223,6 +223,24 @@ class SystemStatus extends Model { return $as_fahrenheit ? $temp * 1.8 + 32 : $temp; } + /** + * Obtains the CPU load average. This method is responsible for obtaining the raw load average string from pfSense + * and ensuring it is formatted correctly before the cpu_load_average field splits it into an array. + * @return string|null The CPU load average as a string or null if it could not be obtained. + */ + private function get_cpu_load_average(): ?string { + # Obtain the load average using pfSense's get_load_average() function + $load_avg = get_load_average(); + + # The system language may affect the load average's delimiter (missing a comma in some languages) + # ensure values are only ever separated by a single space (no commas). #716 + if (str_contains(", ", $load_avg)) { + $load_avg = str_replace(', ', ' ', $load_avg); + } + + return $load_avg ?? null; + } + /** * Determines approximate CPU usage using the last minute load average. * @return float|null The CPU usage as a whole percentage. From 1d96e2751b0baf595de0542349228e790c660501 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Fri, 27 Jun 2025 21:48:43 -0600 Subject: [PATCH 2/5] fix: reverse needle and haystack in str_contains call --- .../files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc index de988ac03..1009e8937 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc @@ -234,7 +234,7 @@ class SystemStatus extends Model { # The system language may affect the load average's delimiter (missing a comma in some languages) # ensure values are only ever separated by a single space (no commas). #716 - if (str_contains(", ", $load_avg)) { + if (str_contains($load_avg, ", ")) { $load_avg = str_replace(', ', ' ', $load_avg); } From 7da247bf4e4f186b3378e64c89857f69568a6050 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Fri, 27 Jun 2025 21:49:47 -0600 Subject: [PATCH 3/5] test: ensure language does not affect cpu_load_average #716 --- .../Tests/APIModelsSystemStatusTestCase.inc | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc index abe83db8e..7887c4521 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc @@ -3,6 +3,7 @@ namespace RESTAPI\Tests; use RESTAPI\Core\Command; +use RESTAPI\Core\Model; use RESTAPI\Core\TestCase; use RESTAPI\Models\SystemStatus; @@ -44,4 +45,33 @@ class APIModelsSystemStatusTestCase extends TestCase { (new Command(command: '/bin/kenv -q smbios.bios.reldate 2>/dev/null', trim_whitespace: true))->output, ); } + + /** + * Checks that the load average can can be read correctly despite the system's assign language. Normally, + * the load average format can change based on the systems assigned language and cause issues parsing the + * value into an array. This is a regression test for #716. + */ + public function test_language_does_not_change_cpu_load_avg(): void { + # Change the language to German + Model::set_config("system/language", "de_DE"); + set_language(); + + # Ensure we can read the SystemStatus cpu_load_average without an error + $this->assert_does_not_throw( + callable: function () { + $system_status = new SystemStatus(); + $this->assert_equals(count($system_status->cpu_load_avg->value), 3); + } + ); + + # Reset the language to English and check again + Model::set_config("system/language", "en_US"); + set_language(); + $this->assert_does_not_throw( + callable: function () { + $system_status = new SystemStatus(); + $this->assert_equals(count($system_status->cpu_load_avg->value), 3); + } + ); + } } From ec5aa7bf420cb8587fed6208b974c3d64c61f5e5 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Fri, 27 Jun 2025 21:51:19 -0600 Subject: [PATCH 4/5] fix: don't include newlines in FloatField error msg --- .../files/usr/local/pkg/RESTAPI/Fields/FloatField.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc index f40e63a82..b27278709 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc @@ -144,8 +144,8 @@ class FloatField extends RESTAPI\Core\Field { # Otherwise, the internal value cannot be represented by this Field. Throw an error. throw new RESTAPI\Responses\ServerError( - message: "Cannot parse FloatField '$this->name' from internal because its internal value is not - a numeric value. Consider changing this field to a StringField.", + message: "Cannot parse FloatField '$this->name' from internal because its internal value is not ". + "a numeric value. Consider changing this field to a StringField.", response_id: 'FLOAT_FIELD_WITH_NON_FLOAT_INTERNAL_VALUE', ); } From 3ec529197adf8ae8f64538161dc07905a7018d32 Mon Sep 17 00:00:00 2001 From: Jared Hendrickson Date: Fri, 27 Jun 2025 21:51:49 -0600 Subject: [PATCH 5/5] style: run prettier on changed files --- .../files/usr/local/pkg/RESTAPI/Fields/FloatField.inc | 4 ++-- .../files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc | 2 +- .../pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc index b27278709..8ffa77be8 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/FloatField.inc @@ -144,8 +144,8 @@ class FloatField extends RESTAPI\Core\Field { # Otherwise, the internal value cannot be represented by this Field. Throw an error. throw new RESTAPI\Responses\ServerError( - message: "Cannot parse FloatField '$this->name' from internal because its internal value is not ". - "a numeric value. Consider changing this field to a StringField.", + message: "Cannot parse FloatField '$this->name' from internal because its internal value is not " . + 'a numeric value. Consider changing this field to a StringField.', response_id: 'FLOAT_FIELD_WITH_NON_FLOAT_INTERNAL_VALUE', ); } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc index 1009e8937..aa1bab0bc 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/SystemStatus.inc @@ -234,7 +234,7 @@ class SystemStatus extends Model { # The system language may affect the load average's delimiter (missing a comma in some languages) # ensure values are only ever separated by a single space (no commas). #716 - if (str_contains($load_avg, ", ")) { + if (str_contains($load_avg, ', ')) { $load_avg = str_replace(', ', ' ', $load_avg); } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc index 7887c4521..c206d8a75 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc +++ b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsSystemStatusTestCase.inc @@ -53,7 +53,7 @@ class APIModelsSystemStatusTestCase extends TestCase { */ public function test_language_does_not_change_cpu_load_avg(): void { # Change the language to German - Model::set_config("system/language", "de_DE"); + Model::set_config('system/language', 'de_DE'); set_language(); # Ensure we can read the SystemStatus cpu_load_average without an error @@ -61,17 +61,17 @@ class APIModelsSystemStatusTestCase extends TestCase { callable: function () { $system_status = new SystemStatus(); $this->assert_equals(count($system_status->cpu_load_avg->value), 3); - } + }, ); # Reset the language to English and check again - Model::set_config("system/language", "en_US"); + Model::set_config('system/language', 'en_US'); set_language(); $this->assert_does_not_throw( callable: function () { $system_status = new SystemStatus(); $this->assert_equals(count($system_status->cpu_load_avg->value), 3); - } + }, ); } }