"Field `id` is required" when patching /api/v2/firewall/rule

[michelsup]

Describe the bug
When you go to /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint, fill an id, and Execute you get a 400 error.

To Reproduce
Go to /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint
Select "application/x-www-form-urlencoded"
Enter an existing rule id
Click Execute

Expected behavior
Get a 200 response

Screenshots or Response
curl -X 'PATCH' \ 'https://xxxxx:4443/api/v2/firewall/rule' \ -H 'accept: application/json' \ -H 'Authorization: Basic YWRtaW46UHdldDY0NTch' \ -H 'Content-Type: application/x-www-form-urlencoded' \ -d 'descr=&gateway=&log=&source_port=&floating=&pdnpipe=&ackqueue=&statetype=keep%20state&tcp_flags_set=&quick=&destination=&defaultqueue=&destination_port=&tcp_flags_any=&icmptype=any&sched=&dnpipe=&protocol=&interface=&type=&source=&id=25&disabled=&tcp_flags_out_of=&ipprotocol=&direction=any'

{ "code": 400, "status": "bad request", "response_id": "MODEL_REQUIRES_ID", "message": "Field id is required.", "data": [] }

pfSense Version & Package Version:

• pfSense Version: 24.03
• Package Version 2.0

Affected Endpoints:

• URL: /api/v2/firewall/rule

[jaredhendrickson13]

The application/x-www-form-urlencoded content-type expects the parameters to be included the URL itself rather than the request body, for example:

curl -s -k -u admin:pfsense -H "content-type: application/x-www-form-urlencoded" -H "accept: application/json" -X PATCH 'https://localhost/api/v2/firewall/rule?id=0'

{
  "code": 200,
  "status": "ok",
  "response_id": "SUCCESS",
  "message": "",
  "data": {
    "id": 0,
    "type": "pass",
    "interface": [
      "lan"
    ],
    "ipprotocol": "inet",
    "protocol": null,
    "icmptype": null,
    "source": "lan",
    "source_port": null,
    "destination": "any",
    "destination_port": null,
    "descr": "Default allow LAN to any rule",
    "disabled": false,
    "log": false,
    "statetype": "keep state",
    "tcp_flags_any": false,
    "tcp_flags_out_of": null,
    "tcp_flags_set": null,
    "gateway": null,
    "sched": null,
    "dnpipe": null,
    "pdnpipe": null,
    "defaultqueue": null,
    "ackqueue": null,
    "floating": false,
    "quick": null,
    "direction": null,
    "tracker": 100000101,
    "created_time": 1713996066,
    "created_by": "",
    "updated_time": 1713996066,
    "updated_by": "admin@127.0.0.1 (API)"
  }
}

However, application/x-www-form-urlencoded is not recommended for requests other than GET and DELETE requests because it only supports type inference and the API requires strict typing (e.g. there's no way to differentiate "true" from true, or "443" from 443). application/json is the recommended content-type for POST, PATCH and PUT requests.

[michelsup]

Thank you for your quick reply. My bad then, I should have miss the info on the documentation.
I would suggest you to remove the ability to use application/x-www-form-urlencoded within the documentation if it's not recommended, and to be more precise, if it plainly doesn't work as expected/suggested in /api/v2/documentation#/FIREWALL/patchFirewallRuleEndpoint.
BTW, thank you for the hint and, overall, your work.

[jaredhendrickson13]

After looking into this a little more, I do think this is a valid issue. I'll keep this open as an enhancement since the API should allow URL encoded parameters to be submitted via request body per HTTP standard. It was also supported in v1 so there's not really a valid reason to have that limitation in v2. I'll make sure to update documentation accordingly as well.

Thanks!