Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121334
MIME-Version: 1.0
References: <CAFPFaM+V96iS33+Sz7nPU2Vht18j+8cabxh3BE4HeN=J66gzLw@mail.gmail.com>
In-Reply-To: <CAFPFaM+V96iS33+Sz7nPU2Vht18j+8cabxh3BE4HeN=J66gzLw@mail.gmail.com>
Date: Mon, 16 Oct 2023 13:49:33 +0100
Message-ID: <CAFPFaMJmtTkmQkMJ2TCCh77zmgzQdwfEeMO02CFJ5ATWg2_ybg@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000003565520607d4d77e"
Subject: Re: [PHP-DEV] Peculiar behaviour of ext/xml set_X_handler() functions
From: george.banyard@gmail.com ("G. P. B.")

--0000000000003565520607d4d77e
Content-Type: text/plain; charset="UTF-8"

On Tue, 10 Oct 2023 at 14:52, G. P. B. <george.banyard@gmail.com> wrote:

> 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
>
> [1] https://wiki.php.net/rfc/deprecations_php_8_4
>

If no one complains I will merge the PR at the end of the week.

Best regards,

George P. Banyard

--0000000000003565520607d4d77e--