Skip to content

ext/snmp: promote invalid-input warnings to ValueError#21319

Merged
Girgias merged 5 commits intophp:masterfrom
prateekbhujel:prateek/fix-snmp-valueerror-21318
Apr 6, 2026
Merged

ext/snmp: promote invalid-input warnings to ValueError#21319
Girgias merged 5 commits intophp:masterfrom
prateekbhujel:prateek/fix-snmp-valueerror-21318

Conversation

@prateekbhujel
Copy link
Copy Markdown
Contributor

This addresses #21318 by promoting remaining invalid-input warning paths in ext/snmp to ValueError for consistency with PHP 8+ argument validation behavior.

Changed paths:

  • Malformed IPv6 host (snmp_session_init)
  • Missing type/value in OID arrays (php_snmp_parse_oid)
  • Invalid contextEngineID (snmp_session_set_contextEngineID)

Tests updated accordingly:

  • ext/snmp/tests/ipv6.phpt
  • ext/snmp/tests/snmpset.phpt
  • ext/snmp/tests/snmp2_set.phpt
  • ext/snmp/tests/snmp-object-setSecurity_error.phpt

@iluuu1994
Copy link
Copy Markdown
Member

Hi @prateekbhujel! Thanks for your contribution. Per new policy, promotions of errors are considered breaking and require a discussion on the internals mailing list and frequently an RFC.

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

@iluuu1994 Thanks for clarifying the new policy. I will start an internals thread for this BC change and link it here before moving this PR forward.

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

@iluuu1994 Thanks again for the guidance. I started the internals discussion here: https://news-web.php.net/php.internals/130231

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

@iluuu1994 Quick update — I noticed the related policy RFC has since been published: https://wiki.php.net/rfc/policy-exempt-type-value-error-bc-policy. I'm waiting for that vote to conclude before proceeding. Happy to move forward once the policy direction is clear.

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

@iluuu1994 The policy RFC has been accepted, so I think this PR fits the current BC policy now. Could you re-review when you get a chance?
RFC: https://wiki.php.net/rfc/policy-exempt-type-value-error-bc-policy

@prateekbhujel prateekbhujel force-pushed the prateek/fix-snmp-valueerror-21318 branch from b177d74 to 4a448a7 Compare April 6, 2026 15:24
@prateekbhujel prateekbhujel requested a review from Girgias April 6, 2026 17:08
Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only final remark I have would be to add UNEXPECTED() compiler hints for the exception code paths. :)

@Girgias
Copy link
Copy Markdown
Member

Girgias commented Apr 6, 2026

I'll wait for CI and merge afterwards :)

@Girgias Girgias merged commit cbe0144 into php:master Apr 6, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants