Skip to content

Commit 192546c

Browse files
tests: add test for excessive memory usage when copying model #617 objects
1 parent 95cadfa commit 192546c

File tree

4 files changed

+107
-5
lines changed

4 files changed

+107
-5
lines changed

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,22 @@ class Model {
357357
});
358358
}
359359

360+
/**
361+
* Creates an unlinked copy of this Model object.
362+
* @return Model The cloned Model object.
363+
*/
364+
public function copy(): Model {
365+
# Create a copy of this object by serializing and unserializing it
366+
$copy = unserialize(serialize($this));
367+
368+
# Reassign the parent Model object if it exists
369+
if ($this->parent_model) {
370+
$copy->parent_model = $this->parent_model;
371+
}
372+
373+
return $copy;
374+
}
375+
360376
/**
361377
* Checks for options passed in during object construction and maps known options to Model properties. Any
362378
* options that are not known options to the core Model will be returned so they can be merged into
@@ -409,7 +425,7 @@ class Model {
409425
if ($this->config_path or $this->internal_callable) {
410426
$this->id = $id;
411427
$this->from_internal();
412-
$this->initial_object = unserialize(serialize($this));
428+
$this->initial_object = $this->copy();
413429
}
414430
}
415431
}
@@ -989,7 +1005,7 @@ class Model {
9891005

9901006
# Obtain the object from its internal form
9911007
$this->from_internal();
992-
$this->initial_object = unserialize(serialize($this));
1008+
$this->initial_object = $this->copy();
9931009
}
9941010

9951011
# Loop through each field in this Model and assign their values using the `representation_data`.
@@ -2028,7 +2044,7 @@ class Model {
20282044
}
20292045

20302046
# Refresh the initial object
2031-
$this->initial_object = unserialize(serialize($this));
2047+
$this->initial_object = $this->copy();
20322048
}
20332049

20342050
# Return the current representation of this object
@@ -2118,7 +2134,7 @@ class Model {
21182134
}
21192135

21202136
# Refresh the initial object
2121-
$this->initial_object = unserialize(serialize($this));
2137+
$this->initial_object = $this->copy();
21222138
}
21232139

21242140
# Return the current representation of this object
@@ -2296,7 +2312,7 @@ class Model {
22962312
}
22972313

22982314
# Refresh the initial object
2299-
$this->initial_object = unserialize(serialize($this));
2315+
$this->initial_object = $this->copy();
23002316
}
23012317

23022318
# Return the current representation of this object

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Tools.inc

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,28 @@ function get_classes_from_namespace(string $namespace, bool $shortnames = false)
168168

169169
return $classes;
170170
}
171+
172+
/**
173+
* Generates a random MAC address string.
174+
* @returns string A random MAC address string.
175+
*/
176+
function generate_mac_address(): string {
177+
# Generate the first three octets (OUI) with fixed bits for unicast and locally administered addresses
178+
$first_octet = dechex((mt_rand(0x00, 0xff) & 0xfe) | 0x02); // Ensure it is locally administered
179+
$mac = [
180+
$first_octet,
181+
dechex(mt_rand(0x00, 0xff)),
182+
dechex(mt_rand(0x00, 0xff)),
183+
dechex(mt_rand(0x00, 0xff)),
184+
dechex(mt_rand(0x00, 0xff)),
185+
dechex(mt_rand(0x00, 0xff)),
186+
];
187+
188+
# Zero-pad single-character hex values and return the MAC address
189+
return implode(
190+
':',
191+
array_map(function ($part) {
192+
return str_pad($part, 2, '0', STR_PAD_LEFT);
193+
}, $mac),
194+
);
195+
}

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreModelTestCase.inc

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@ use RESTAPI;
77
use RESTAPI\Caches\RESTAPIVersionReleasesCache;
88
use RESTAPI\Core\Auth;
99
use RESTAPI\Core\Model;
10+
use RESTAPI\Models\DHCPServer;
11+
use RESTAPI\Models\DHCPServerStaticMapping;
1012
use RESTAPI\Models\FirewallAlias;
1113
use RESTAPI\Models\InterfaceVLAN;
1214
use RESTAPI\Models\SystemStatus;
1315
use RESTAPI\Models\Test;
16+
use function RESTAPI\Models\DHCPServerStaticMapping;
1417

1518
class APICoreModelTestCase extends RESTAPI\Core\TestCase {
1619
/**
@@ -1197,4 +1200,52 @@ class APICoreModelTestCase extends RESTAPI\Core\TestCase {
11971200
# Remove the write lock
11981201
unlink(Model::WRITE_LOCK_FILE);
11991202
}
1203+
1204+
/**
1205+
* Ensure the copy() method creates an unlinked copy of the Model object.
1206+
*/
1207+
public function test_copy(): void {
1208+
# Create a new FirewallAlias model to test with
1209+
$alias = new FirewallAlias(name: 'test_alias', type: 'host', address: ['1.2.3.4']);
1210+
$alias->id = 5;
1211+
$alias_copy = $alias->copy();
1212+
1213+
# Ensure the values are equal
1214+
$this->assert_equals($alias->to_internal(), $alias_copy->to_internal());
1215+
$this->assert_equals($alias_copy->id, 5);
1216+
$this->assert_equals($alias->to_representation(), $alias_copy->to_representation());
1217+
1218+
# Change the copy's values and ensure the original values are not changed
1219+
$alias_copy->name->value = 'new_name';
1220+
$this->assert_not_equals($alias->name->value, $alias_copy->name->value);
1221+
}
1222+
1223+
/**
1224+
* Ensure the copy() method does not use excessive memory in larger datasets. This is a regression test for #617.
1225+
*/
1226+
public function test_copy_memory_usage(): void {
1227+
# Generate lots of DHCP Server Static Mappings
1228+
$static_mappings = [];
1229+
$used_macs = [];
1230+
while (count($static_mappings) < 450) {
1231+
# Generate a unique mac address for each static mapping
1232+
$mac = RESTAPI\Core\Tools\generate_mac_address();
1233+
if (in_array($mac, $used_macs)) {
1234+
continue;
1235+
}
1236+
$static_mappings[] = ['mac' => $mac];
1237+
}
1238+
1239+
# Update the DHCP server with our static mappings
1240+
$dhcp_server = new DHCPServer(id: 'lan', staticmap: $static_mappings);
1241+
$dhcp_server->update();
1242+
1243+
# Read a static mapping and copy it
1244+
$static_mapping = new DHCPServerStaticMapping(id: 50, parent_id: 'lan');
1245+
$static_mapping->copy();
1246+
1247+
# Remove the static mappings
1248+
$dhcp_server->staticmap->value = [];
1249+
$dhcp_server->update(apply: true);
1250+
}
12001251
}

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreToolsTestCase.inc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require_once 'RESTAPI/autoloader.inc';
66

77
use RESTAPI\Core\TestCase;
88
use function RESTAPI\Core\Tools\bandwidth_to_bits;
9+
use function RESTAPI\Core\Tools\generate_mac_address;
910
use function RESTAPI\Core\Tools\is_assoc_array;
1011
use function RESTAPI\Core\Tools\to_upper_camel_case;
1112

@@ -52,4 +53,13 @@ class APICoreToolsTestCase extends TestCase {
5253
$this->assert_equals(to_upper_camel_case('this is a test'), 'ThisIsATest');
5354
$this->assert_equals(to_upper_camel_case('_this is-a test'), 'ThisIsATest');
5455
}
56+
57+
/**
58+
* Checks that the generate_mac_address() function correctly generates a random MAC address.
59+
*/
60+
public function test_generate_mac_address(): void {
61+
$mac_address = generate_mac_address();
62+
$this->assert_equals(strlen($mac_address), 17);
63+
$this->assert_is_true(preg_match('/^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$/', $mac_address) === 1);
64+
}
5565
}

0 commit comments

Comments
 (0)