From 0e3caf1ac48e94d6d2835cc8cbdab6a6b5caf945 Mon Sep 17 00:00:00 2001 From: Monviech <79600909+Monviech@users.noreply.github.com> Date: Sat, 1 Jun 2024 18:51:24 +0200 Subject: [PATCH] www/caddy: Fix crash of searchAction when reverseUuids is null (#4016) * Fix crash when reverseUuids is null. Improve performance by using associative arrays and isset instead of in_array. * Refactored search into a helper function that is called in each section. Made comments clearer. Moved helper functions to top of file before the boilerplate sections. * Generalize the search helper function a bit more. Now any sections UUID can be referenced, or null for its own UUID. * Generalize the searchActionHelper function completely. --- .../Caddy/Api/ReverseProxyController.php | 147 +++++++++--------- 1 file changed, 76 insertions(+), 71 deletions(-) diff --git a/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/Api/ReverseProxyController.php b/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/Api/ReverseProxyController.php index de6b82b64..5f32443d0 100644 --- a/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/Api/ReverseProxyController.php +++ b/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/Api/ReverseProxyController.php @@ -39,24 +39,74 @@ class ReverseProxyController extends ApiMutableModelControllerBase protected static $internalModelClass = 'OPNsense\Caddy\Caddy'; protected static $internalModelUseSafeDelete = true; - /*ReverseProxy Section*/ - - /*Search Function adjusted for the search filter dropdown*/ - public function searchReverseProxyAction() + /** + * Function for search filter dropdown + * + * @return array containing rows of domain and port combinations. + */ + public function getAllReverseDomainsAction() { - // Get a comma-separated list of UUIDs from the request - $reverseUuids = $this->request->get('reverseUuids'); - $uuidArray = !empty($reverseUuids) ? explode(',', $reverseUuids) : []; + $this->sessionClose(); // Close session early for performance + $result = array("rows" => array()); - // Define the filter function to handle multiple UUIDs - $filterFunction = function ($modelItem) use ($uuidArray) { - $itemUuid = (string)$modelItem->getAttributes()['uuid']; - // Include the item if no UUIDs are provided (empty array) or if it's in the array of UUIDs - return empty($uuidArray) || in_array($itemUuid, $uuidArray, true); + $mdlCaddy = new \OPNsense\Caddy\Caddy(); + $reverseNodes = $mdlCaddy->reverseproxy->reverse->iterateItems(); + + foreach ($reverseNodes as $item) { + if (!empty($item->FromDomain)) { + // Conditionally concatenate port if it exists + $domain = (string)$item->FromDomain; + $port = (string)$item->FromPort; + $combinedDomainPort = $domain . (!empty($port) ? ':' . $port : ''); + + $result['rows'][] = array( + 'id' => (string)$item->getAttributes()['uuid'], + 'domainPort' => $combinedDomainPort // Combined domain and port, conditionally adding port + ); + } + } + + return $result; + } + + /** + * Centralized and generalized helper function for searching across different sections of the reverse proxy setup. + * This function mostly helps when model relation fields are used. + * It filters entries based on UUIDs provided as an argument. The section or key used for the UUID + * can be specified, allowing for direct or indirect UUID referencing. + * + * @param string $modelPath The data model path identifier, pointing to the section of the model being searched. + * @param string $uuidSearchBase The request parameter name for the comma-separated list of UUIDs to filter the search results. + * @param string|null $uuidReferenceKey The specific attribute key used to fetch the UUID for filtering. If null, defaults to the item's own UUID. + * @return array Filtered search results. + */ + private function searchActionHelper($modelPath, $uuidSearchBase, $uuidReferenceKey = null) + { + // Fetch the comma-separated UUIDs string from the request using the provided parameter name. + $uuidList = $this->request->get($uuidSearchBase); + // Ensure the retrieved UUID list is a string and not empty before attempting to explode it. + $uuidArray = (!empty($uuidList) && is_string($uuidList)) ? explode(',', $uuidList) : []; + + // Define a filter function to determine which items to include based on the UUID. + $filterFunction = function ($modelItem) use ($uuidArray, $uuidReferenceKey) { + // Extract UUID from the item, using the specified UUID key if provided, otherwise default to direct UUID access. + $modelUUID = ($uuidReferenceKey !== null) ? (string)$modelItem->$uuidReferenceKey : (string)$modelItem->getAttributes()['uuid']; + // Include the item if the UUID array is empty or if the item's UUID is in the array. + return empty($uuidArray) || in_array($modelUUID, $uuidArray, true); }; - // Return the search results filtered by the provided UUIDs, if any - return $this->searchBase("reverseproxy.reverse", null, 'description', $filterFunction); + // Perform the search using the specified model path and the filter function, returning the results. + // Note: This uses the existing search function of the ApiMutableModelControllerBase + return $this->searchBase($modelPath, null, 'description', $filterFunction); + } + + + // ReverseProxy Section + + public function searchReverseProxyAction() + { + // For domains, use their domain UUIDs directly, $uuidReferenceKey null added for explicit clarity + return $this->searchActionHelper("reverseproxy.reverse", "reverseUuids", null); } public function setReverseProxyAction($uuid) @@ -84,47 +134,14 @@ class ReverseProxyController extends ApiMutableModelControllerBase return $this->toggleBase("reverseproxy.reverse", $uuid, $enabled); } - /*Function for the search filter dropdown in the bootgrid*/ - public function getAllReverseDomainsAction() - { - $this->sessionClose(); // Close session early for performance - $result = array("rows" => array()); - $mdlCaddy = new \OPNsense\Caddy\Caddy(); - $reverseNodes = $mdlCaddy->reverseproxy->reverse->iterateItems(); + // Subdomain Section - foreach ($reverseNodes as $item) { - if (!empty($item->FromDomain)) { - // Conditionally concatenate port if it exists - $domain = (string)$item->FromDomain; - $port = (string)$item->FromPort; - $combinedDomainPort = $domain . (!empty($port) ? ':' . $port : ''); - - $result['rows'][] = array( - 'id' => (string)$item->getAttributes()['uuid'], - 'domainPort' => $combinedDomainPort // Combined domain and port, conditionally adding port - ); - } - } - - return $result; - } - - - /*Subdomain Section*/ - - /*Search Function adjusted for the search filter dropdown*/ public function searchSubdomainAction() { - $reverseUuids = $this->request->get('reverseUuids'); - $uuidArray = !empty($reverseUuids) ? explode(',', $reverseUuids) : []; - - $filterFunction = function ($modelItem) use ($uuidArray) { - // Filtering on domain UUIDs referenced by subdomains - return empty($uuidArray) || in_array((string)$modelItem->reverse, $uuidArray, true); - }; - - return $this->searchBase("reverseproxy.subdomain", null, 'description', $filterFunction); + // For subdomains, compare 'reverseUuids' (which contain domain UUIDs) + // to 'reverse' (which contain the same domain UUIDs due to model relation field) + return $this->searchActionHelper("reverseproxy.subdomain", "reverseUuids", "reverse"); } public function setSubdomainAction($uuid) @@ -153,26 +170,14 @@ class ReverseProxyController extends ApiMutableModelControllerBase } - /*Handler Section*/ + // Handler Section - /*Search Function adjusted for the search filter dropdown*/ + // Adjusted for search filter dropdown, using helper function public function searchHandleAction() { - $reverseUuids = $this->request->get('reverseUuids'); - $uuidArray = explode(',', $reverseUuids); - - if (empty($reverseUuids)) { - // If no UUIDs are provided, do not apply any filter, return all records - return $this->searchBase("reverseproxy.handle", null, 'description'); - } else { - // Apply the filter only if UUIDs are provided - $filterFunction = function ($modelItem) use ($uuidArray) { - $modelUUID = (string)$modelItem->reverse; - return in_array($modelUUID, $uuidArray, true); - }; - - return $this->searchBase("reverseproxy.handle", null, 'description', $filterFunction); - } + // For handles, compare 'reverseUuids' (which contain domain UUIDs) + // to 'reverse' (which contain the same domain UUIDs due to model relation field) + return $this->searchActionHelper("reverseproxy.handle", "reverseUuids", "reverse"); } public function setHandleAction($uuid) @@ -201,7 +206,7 @@ class ReverseProxyController extends ApiMutableModelControllerBase } - /* AccessList Section */ + // AccessList Section public function searchAccessListAction() { @@ -229,7 +234,7 @@ class ReverseProxyController extends ApiMutableModelControllerBase } - /* BasicAuth Section */ + // BasicAuth Section public function searchBasicAuthAction() { @@ -277,7 +282,7 @@ class ReverseProxyController extends ApiMutableModelControllerBase } - /* Header Section */ + // Header Section public function searchHeaderAction() {