diff --git a/ACL_DROPDOWN_BUG_FIX.md b/ACL_DROPDOWN_BUG_FIX.md index a96345e0..aaa91512 100644 --- a/ACL_DROPDOWN_BUG_FIX.md +++ b/ACL_DROPDOWN_BUG_FIX.md @@ -1,4 +1,4 @@ -# ACL Dropdown Bug Fix - RESOLVED +# ACL & Security Headers Dropdown Bug Fix - RESOLVED **Date**: February 12, 2026 **Status**: ✅ FIXED @@ -9,221 +9,248 @@ ## User Report > "There is a bug in the ACL dropdown menu. I could not remove or edit the attached ACL on a proxy host. I had to delete the host and add it without the ACL to bypass." +> +> "Same issue with Security Headers dropdown - cannot remove or change once selected." -**Impact**: Users unable to manage ACL assignments on proxy hosts, forcing deletion and recreation. +**Impact**: Users unable to manage ACL or Security Headers on proxy hosts, forcing deletion and recreation. --- ## Root Cause Analysis -### The Problem +### The REAL Problem: Stale Closure Bug -The `AccessListSelector` component had a **controlled component pattern bug** in the value mapping: +The bug was **NOT** in `AccessListSelector.tsx` - that component was correctly implemented. + +The bug was in **`ProxyHostForm.tsx`** where state updates used stale closures: + +```jsx +// ❌ BUGGY CODE (Line 822 & 836) + setFormData({ ...formData, access_list_id: id })} +/> -```typescript -// ❌ BUGGY CODE ``` -#### Critical Issues Identified +### Critical Issues -1. **Ambiguous Value Mapping**: - - When `value = null` (no ACL), component shows `"0"` - - When user clicks "No Access Control" (value="0"), it may not trigger change - - Expression `parseInt("0") || null` relies on falsy coercion, creating edge case bugs +1. **Stale Closure Pattern**: + - `onChange` callback captures `formData` from render when created + - When callback executes, `formData` may be stale + - Spreading stale `formData` overwrites current state with old values + - Only the changed field updates, but rest of form may revert -2. **Lack of Explicit Null Handling**: - - No clear distinction between "no selection" and "explicitly selecting none" - - Controlled component may not recognize `"0"` → `"0"` as a state change +2. **React setState Batching**: + - React batches multiple setState calls for performance + - If multiple updates happen quickly, closure captures old state + - Direct object spread `{ ...formData, ... }` uses stale snapshot -3. **Potential Re-render Issues**: - - Select component might optimize away updates when value appears unchanged - - `String(value || 0)` can produce same string for different logical states +3. **No Re-render Trigger**: + - State update happens but uses old values + - Select component receives same value, doesn't re-render + - User sees no change, thinks dropdown is broken --- ## The Fix -### Solution: Explicit "none" Value +### Solution: Functional setState Form -```typescript +```jsx // ✅ FIXED CODE -export default function AccessListSelector({ value, onChange }: AccessListSelectorProps) { - const { data: accessLists } = useAccessLists(); - const selectedACL = accessLists?.find((acl) => acl.id === value); + setFormData(prev => ({ ...prev, access_list_id: id }))} +/> - // Convert between component's string-based value and the prop's number|null - const selectValue = value === null || value === undefined ? 'none' : String(value); - - const handleValueChange = (newValue: string) => { - if (newValue === 'none') { - onChange(null); - } else { - const numericId = parseInt(newValue, 10); - if (!isNaN(numericId)) { - onChange(numericId); - } - } - }; - - return ( - - ); -} + setFormData({ ...formData, forward_scheme: scheme })}> + setFormData({ ...formData, certificate_id: parseInt(e) || null })}> + setFormData({ ...formData, ssl_forced: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, ssl_forced: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> Force SSL @@ -1108,7 +1108,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, http2_support: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, http2_support: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> HTTP/2 Support @@ -1120,7 +1120,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, hsts_enabled: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, hsts_enabled: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> HSTS Enabled @@ -1132,7 +1132,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, hsts_subdomains: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, hsts_subdomains: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> HSTS Subdomains @@ -1144,7 +1144,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, block_exploits: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, block_exploits: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> Block Exploits @@ -1156,7 +1156,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, websocket_support: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, websocket_support: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> Websockets Support @@ -1168,7 +1168,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor setFormData({ ...formData, enable_standard_headers: e.target.checked })} + onChange={e => setFormData(prev => ({ ...prev, enable_standard_headers: e.target.checked }))} className="w-4 h-4 text-blue-600 bg-gray-900 border-gray-700 rounded focus:ring-blue-500" /> Enable Standard Proxy Headers @@ -1214,7 +1214,7 @@ export default function ProxyHostForm({ host, onSubmit, onCancel }: ProxyHostFor