From fec096874e1e7570c6f62d8c1dc5266aa6ea3972 Mon Sep 17 00:00:00 2001 From: "Joseph.Rawlings" Date: Mon, 1 Dec 2025 13:30:11 -0500 Subject: [PATCH] refactor: improve payload construction and error handling in Proxmox Firewall component for enhanced clarity and robustness --- .../Servers/Proxmox/Livewire/Firewall.php | 66 +++++++++++++++---- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/extensions/Servers/Proxmox/Livewire/Firewall.php b/extensions/Servers/Proxmox/Livewire/Firewall.php index 9ca24e169..f3592ce5f 100644 --- a/extensions/Servers/Proxmox/Livewire/Firewall.php +++ b/extensions/Servers/Proxmox/Livewire/Firewall.php @@ -72,7 +72,7 @@ class Firewall extends Component 'verify' => true, ]); - // For PUT/POST requests, Proxmox API typically expects form data, not JSON + // For PUT/POST requests, Proxmox API expects form data if (in_array(strtolower($method), ['put', 'post']) && !empty($data)) { return $request->asForm()->$method($fullUrl, $data)->throw(); } @@ -231,19 +231,43 @@ class Firewall extends Component { $this->loading = true; try { - $payload = array_filter([ + // Build payload - always include action, enable, and type + // Type defaults to 'in' for incoming rules (can be 'in' or 'out') + $payload = [ 'action' => $this->ruleAction, - 'source' => $this->ruleSource ?: null, - 'dest' => $this->ruleDest ?: null, - 'proto' => $this->ruleProto ?: null, - 'dport' => $this->ruleDport ?: null, - 'sport' => $this->ruleSport ?: null, - 'comment' => $this->ruleComment ?: null, 'enable' => $this->ruleEnable ? 1 : 0, - 'macro' => $this->ruleMacro ?: null, - 'iface' => $this->ruleIface ?: null, - 'log' => $this->ruleLog ?: null, - ], fn($value) => $value !== null); + 'type' => 'in', // Default to incoming rule + ]; + + // Add optional parameters only if they have values + // Note: Proxmox allows rules with just action, but some parameters may be required in certain contexts + if (!empty(trim($this->ruleSource))) { + $payload['source'] = trim($this->ruleSource); + } + if (!empty(trim($this->ruleDest))) { + $payload['dest'] = trim($this->ruleDest); + } + if (!empty(trim($this->ruleProto))) { + $payload['proto'] = trim($this->ruleProto); + } + if (!empty(trim($this->ruleDport))) { + $payload['dport'] = trim($this->ruleDport); + } + if (!empty(trim($this->ruleSport))) { + $payload['sport'] = trim($this->ruleSport); + } + if (!empty(trim($this->ruleComment))) { + $payload['comment'] = trim($this->ruleComment); + } + if (!empty(trim($this->ruleMacro))) { + $payload['macro'] = trim($this->ruleMacro); + } + if (!empty(trim($this->ruleIface))) { + $payload['iface'] = trim($this->ruleIface); + } + if (!empty(trim($this->ruleLog))) { + $payload['log'] = trim($this->ruleLog); + } if ($this->editingRulePos !== null) { // Update existing rule @@ -274,12 +298,26 @@ class Firewall extends Component } } catch (\Exception $e) { $errorMessage = $e->getMessage(); + $fullErrorDetails = ''; if (method_exists($e, 'response') && $e->response()) { $errorData = $e->response()->json(); - if (isset($errorData['errors']) && is_array($errorData['errors']) && !empty($errorData['errors'])) { - $errorMessage = $errorData['errors'][0]['message'] ?? $errorMessage; + if (isset($errorData['errors']) && is_array($errorData['errors'])) { + // Try to get the full error details + $errorMessages = []; + foreach ($errorData['errors'] as $error) { + if (is_array($error)) { + $errorMessages[] = json_encode($error); + } else { + $errorMessages[] = $error; + } + } + $fullErrorDetails = implode(' ', $errorMessages); + $errorMessage = !empty($fullErrorDetails) ? $fullErrorDetails : ($errorData['errors'][0]['message'] ?? $errorMessage); } elseif (isset($errorData['message'])) { $errorMessage = $errorData['message']; + if (isset($errorData['errors'])) { + $errorMessage .= ' ' . json_encode($errorData['errors']); + } } } $this->error = 'Failed to save rule: ' . $errorMessage;