Hello internals,
While doing some refactoring of ext/xml to bring it up to modern engine
standards, I discovered some peculiar behaviour of the functions which set
callable handlers.
The PR in question is: https://github.com/php/php-src/pull/12340
The issue is that this function doesn't just accept the usual callabes,
but also passing a method name that should be called on an object provided
by xml_set_object().
Moreover, contrary to the usual semantics of checking if the value is
callable when passed to the function, it is checked when attempting to call
it.
The PR changes this to validate that the function or method name provided
is actually callable.
This requires a BC break in that xml_set_object() MUST be called prior to
setting the method names, otherwise a ValueError is now thrown.
The only project I could find that called xml_set_object() after already
setting method names for handlers is semsol/arc2
https://github.com/semsol/arc2 to which I have sent a PR to update this.
The existence of the already set methods is now also checked when calling
xml_set_object() again with a different object to ensure that the handlers
are always well-defined.
The plan is to also deprecate this feature out right, but this can be done
by adding it to the mass 8.4 deprecation RFC. [1]
Point of this email is to know if anyone has any complaints about this BC
break before merging the PR into master.
Best regards,
George P. Banyard
Hello internals,
While doing some refactoring of ext/xml to bring it up to modern engine
standards, I discovered some peculiar behaviour of the functions which set
callable handlers.The PR in question is: https://github.com/php/php-src/pull/12340
The issue is that this function doesn't just accept the usual callabes,
but also passing a method name that should be called on an object provided
byxml_set_object().Moreover, contrary to the usual semantics of checking if the value is
callable when passed to the function, it is checked when attempting to call
it.
The PR changes this to validate that the function or method name provided
is actually callable.
This requires a BC break in thatxml_set_object()MUST be called prior to
setting the method names, otherwise a ValueError is now thrown.The only project I could find that called
xml_set_object()after already
setting method names for handlers is semsol/arc2
https://github.com/semsol/arc2 to which I have sent a PR to update this.The existence of the already set methods is now also checked when calling
xml_set_object()again with a different object to ensure that the handlers
are always well-defined.The plan is to also deprecate this feature out right, but this can be done
by adding it to the mass 8.4 deprecation RFC. [1]Point of this email is to know if anyone has any complaints about this BC
break before merging the PR into master.Best regards,
George P. Banyard
If no one complains I will merge the PR at the end of the week.
Best regards,
George P. Banyard