From 70e7695b3dbb464545063bd16c65b136ffe2865d Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Wed, 11 Dec 2024 22:22:40 +0100
Subject: [PATCH] model:BaseListField - memory preservation fix, closes
 https://github.com/opnsense/core/issues/8123

remove overhead in BaseListField by passing references to the data with a CallbackValidator.
---
 plist                                         |  2 -
 .../Base/FieldTypes/BaseListField.php         | 33 +++++-----
 .../OPNsense/Base/FieldTypes/PortField.php    |  3 +-
 .../Base/Validators/CsvListValidator.php      | 62 -------------------
 .../OPNsense/Base/Validators/InclusionIn.php  | 57 -----------------
 .../Base/FieldTypes/AuthGroupFieldTest.php    |  4 +-
 .../AuthenticationServerFieldTest.php         |  4 +-
 .../Base/FieldTypes/CertificateFieldTest.php  |  4 +-
 .../Base/FieldTypes/CountryFieldTest.php      |  4 +-
 .../Base/FieldTypes/InterfaceFieldTest.php    |  6 +-
 .../FieldTypes/ModelRelationFieldTest.php     | 12 ++--
 11 files changed, 35 insertions(+), 156 deletions(-)
 delete mode 100644 src/opnsense/mvc/app/models/OPNsense/Base/Validators/CsvListValidator.php
 delete mode 100644 src/opnsense/mvc/app/models/OPNsense/Base/Validators/InclusionIn.php

diff --git a/plist b/plist
index 8e733cc12..fd3f39eea 100644
--- a/plist
+++ b/plist
@@ -637,9 +637,7 @@
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validation.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/ValidationException.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/CallbackValidator.php
-/usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/CsvListValidator.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/Email.php
-/usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/InclusionIn.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/IntegerValidator.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/MinMaxValidator.php
 /usr/local/opnsense/mvc/app/models/OPNsense/Base/Validators/NetworkValidator.php
diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseListField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseListField.php
index 4f906d010..c385b1ab7 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseListField.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseListField.php
@@ -28,8 +28,8 @@
 
 namespace OPNsense\Base\FieldTypes;
 
-use OPNsense\Base\Validators\CsvListValidator;
-use OPNsense\Base\Validators\InclusionIn;
+use OPNsense\Base\Validators\CallbackValidator;
+
 
 /**
  * Class BaseListField
@@ -148,20 +148,21 @@ abstract class BaseListField extends BaseField
     {
         $validators = parent::getValidators();
         if ($this->internalValue != null) {
-            $args = [
-                'domain' => [],
-                'message' => $this->getValidationMessage(),
-            ];
-            foreach (array_keys($this->internalOptionList) as $key) {
-                $args['domain'][] = (string)$key;
-            }
-            if ($this->internalMultiSelect) {
-                // field may contain more than one option
-                $validators[] = new CsvListValidator($args);
-            } else {
-                // single option selection
-                $validators[] = new InclusionIn($args);
-            }
+            $that = $this;
+            $validators[] = new CallbackValidator(["callback" => function ($data) use ($that) {
+                $messages = [];
+                if ($that->internalMultiSelect) {
+                    foreach (explode(",", $data) as $valItem) {
+                        if (!isset($this->internalOptionList[$valItem])) {
+                            $messages[] = $this->getValidationMessage();
+                            break;
+                        }
+                    }
+                } elseif (!isset($this->internalOptionList[$data])) {
+                    $messages[] = $this->getValidationMessage();
+                }
+                return $messages;
+            }]);
         }
         return $validators;
     }
diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/PortField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/PortField.php
index 2eb74b0dd..30f6f6460 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/PortField.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/PortField.php
@@ -203,8 +203,7 @@ class PortField extends BaseListField
     }
 
     /**
-     * retrieve field validators for this field type
-     * @return array returns InclusionIn validator
+     * {@inheritdoc}
      */
     public function getValidators()
     {
diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/Validators/CsvListValidator.php b/src/opnsense/mvc/app/models/OPNsense/Base/Validators/CsvListValidator.php
deleted file mode 100644
index c16ea0f14..000000000
--- a/src/opnsense/mvc/app/models/OPNsense/Base/Validators/CsvListValidator.php
+++ /dev/null
@@ -1,62 +0,0 @@
-<?php
-
-/*
- * Copyright (C) 2015 Deciso B.V.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright notice,
- *    this list of conditions and the following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
- * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
- * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
-
-namespace OPNsense\Base\Validators;
-
-use OPNsense\Base\BaseValidator;
-use OPNsense\Base\Messages\Message;
-
-/**
- * Class CsvListValidator validate a string list against a list of options
- * @package OPNsense\Base\Validators
- */
-class CsvListValidator extends BaseValidator
-{
-    /**
-     * Executes validation
-     *
-     * @param $validator
-     * @param string $attribute
-     * @return boolean
-     */
-    public function validate($validator, $attribute): bool
-    {
-        $value = $validator->getValue($attribute);
-        $domain = $this->getOption('domain');
-        $msg = $this->getOption('message');
-
-        foreach (explode(",", $value) as $valItem) {
-            if (!in_array($valItem, $domain)) {
-                $validator->appendMessage(new Message($msg, $attribute, 'CsvListValidator'));
-                return false;
-            }
-        }
-
-        return true;
-    }
-}
diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/Validators/InclusionIn.php b/src/opnsense/mvc/app/models/OPNsense/Base/Validators/InclusionIn.php
deleted file mode 100644
index c91f2a3fe..000000000
--- a/src/opnsense/mvc/app/models/OPNsense/Base/Validators/InclusionIn.php
+++ /dev/null
@@ -1,57 +0,0 @@
-<?php
-
-/*
- * Copyright (C) 2015 Deciso B.V.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright notice,
- *    this list of conditions and the following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
- * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
- * AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
- * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- */
-
-namespace OPNsense\Base\Validators;
-
-use OPNsense\Base\BaseValidator;
-use OPNsense\Base\Messages\Message;
-
-/**
- * @package OPNsense\Base\Validators
- */
-class InclusionIn extends BaseValidator
-{
-    /**
-     * Executes validation
-     *
-     * @param $validator
-     * @param string $attribute
-     * @return boolean
-     */
-    public function validate($validator, $attribute): bool
-    {
-        $value = $validator->getValue($attribute);
-        $domain = $this->getOption('domain');
-        $msg = $this->getOption('message');
-        if (!in_array($value, $domain)) {
-            $validator->appendMessage(new Message($msg, $attribute, 'InclusionIn'));
-            return false;
-        }
-        return true;
-    }
-}
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthGroupFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthGroupFieldTest.php
index d78a2f34a..aae213bcd 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthGroupFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthGroupFieldTest.php
@@ -71,7 +71,7 @@ class AuthGroupFieldTest extends Field_Framework_TestCase
     public function testSelectSetWithUnknownValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("CsvListValidator");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new AuthGroupField();
         $field->eventPostLoading();
@@ -100,7 +100,7 @@ class AuthGroupFieldTest extends Field_Framework_TestCase
     public function testSelectSetOnSingleValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new AuthGroupField();
         $field->eventPostLoading();
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthenticationServerFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthenticationServerFieldTest.php
index ee1af54fa..e54339efb 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthenticationServerFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/AuthenticationServerFieldTest.php
@@ -84,7 +84,7 @@ class AuthenticationServerFieldTest extends Field_Framework_TestCase
     public function testSelectSetWithUnknownValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("CsvListValidator");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new AuthenticationServerField();
         $field->eventPostLoading();
@@ -113,7 +113,7 @@ class AuthenticationServerFieldTest extends Field_Framework_TestCase
     public function testSelectSetOnSingleValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new AuthenticationServerField();
         $field->eventPostLoading();
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
index b75d07608..eccd09134 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CertificateFieldTest.php
@@ -84,7 +84,7 @@ class CertificateFieldTest extends Field_Framework_TestCase
     public function testSelectSetWithUnknownValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("CsvListValidator");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new CertificateField();
         $field->eventPostLoading();
@@ -113,7 +113,7 @@ class CertificateFieldTest extends Field_Framework_TestCase
     public function testSelectSetOnSingleValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new CertificateField();
         $field->eventPostLoading();
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CountryFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CountryFieldTest.php
index f41d8f528..15e6cd3ec 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CountryFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/CountryFieldTest.php
@@ -102,7 +102,7 @@ class CountryFieldTest extends Field_Framework_TestCase
     public function testSelectSetWithUnknownValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("CsvListValidator");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new CountryField();
         $field->eventPostLoading();
@@ -145,7 +145,7 @@ class CountryFieldTest extends Field_Framework_TestCase
     public function testSelectSetOnSingleValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new CountryField();
         $field->eventPostLoading();
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/InterfaceFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/InterfaceFieldTest.php
index 191ef2eda..9eea596b4 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/InterfaceFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/InterfaceFieldTest.php
@@ -71,7 +71,7 @@ class InterfaceFieldTest extends Field_Framework_TestCase
     public function testSelectHasNoParents()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new InterfaceField();
         $field->eventPostLoading();
@@ -112,7 +112,7 @@ class InterfaceFieldTest extends Field_Framework_TestCase
     public function testSelectSetWithUnknownValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("CsvListValidator");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new InterfaceField();
         $field->eventPostLoading();
@@ -141,7 +141,7 @@ class InterfaceFieldTest extends Field_Framework_TestCase
     public function testSelectSetOnSingleValue()
     {
         $this->expectException(\OPNsense\Base\ValidationException::class);
-        $this->expectExceptionMessage("InclusionIn");
+        $this->expectExceptionMessage("CallbackValidator");
         // init field
         $field = new InterfaceField();
         $field->eventPostLoading();
diff --git a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/ModelRelationFieldTest.php b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/ModelRelationFieldTest.php
index 2719a34f4..5d11a02b2 100644
--- a/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/ModelRelationFieldTest.php
+++ b/src/opnsense/mvc/tests/app/models/OPNsense/Base/FieldTypes/ModelRelationFieldTest.php
@@ -86,7 +86,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("5ea2a35c-b02b-485a-912b-d077e639bf9f,60e1bc02-6817-4940-bbd3-61d0cf439a8a");
-        $this->assertEquals($this->validate($field), ['InclusionIn']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**
@@ -142,7 +142,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("'',5ea2a35c-b02b-485a-912b-d077e639bf9f");
-        $this->assertEquals($this->validate($field), ['CsvListValidator']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**
@@ -184,7 +184,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("Not an option");
-        $this->assertEquals($this->validate($field), ['CsvListValidator']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**
@@ -267,7 +267,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("XX5ea2a35c-b02b-485a-912b-d077e639bf9f");
-        $this->assertEquals($this->validate($field), ['InclusionIn']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**
@@ -305,7 +305,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("x4d0e2835-7a19-4a19-8c23-e12383827594,5ea2a35c-b02b-485a-912b-d077e639bf9f");
-        $this->assertEquals($this->validate($field), ['CsvListValidator']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**
@@ -324,7 +324,7 @@ class ModelRelationFieldTest extends Field_Framework_TestCase
         ));
         $field->eventPostLoading();
         $field->setValue("4d0e2835-7a19-4a19-8c23-e12383827594,5ea2a35c-b02b-485a-912b-d077e639bf9f");
-        $this->assertEquals($this->validate($field), ['InclusionIn']);
+        $this->assertEquals($this->validate($field), ['CallbackValidator']);
     }
 
     /**