On Wed, Aug 21, 2024, at 10:23, John Coggeshall wrote:
>
>
> On Aug 2 2024, at 4:37 pm, Bilge <bi...@scriptfusion.com> wrote:
>> My only concern is there needs to be an alternative way to do this:
>> intercepting internal calls. Sometimes, whether due to poor architecture or
>> otherwise, we just need to be able to replace an internal function call. One
>> example I can think of recently is where I had to replace `header()` with a
>> void function in tests, just to stop some legacy code emitting headers
>> before the main framework kicked in, then unable to emit its own response
>> because HTTP headers had already been sent. In a perfect world it shouldn't
>> be necessary, but sometimes it is, so I think for this proposal to be
>> palpable there must still be a way to achieve this.
>
> Just a tangent thought to the above, but I've always been a little concerned
> with the idea that a malicious composer package could potentially do nasty
> things because PHP looks at the local namespace first for functions. For
> example, if a composer package focused on Laravel that defines malicious
> versions of internal functions for common namespaces like `App\Models` ,
> `App\Http\Controllers` , etc. it could do some nasty stuff -- and
> supply-chain attacks aren't exactly uncommon. Even worse is Wordpress or any
> other PHP-based software package that allows arbitrary plugins to be
> installed by non-technical users who really would have no idea if the package
> was safe even if they were looking at the code.
>
> <?php
> // something.php
> namespace App\Models;
>
> function password_hash(string $password, string|int|null $algo, array
> $options = []): string
> {
> print("Hello");
> return $password;
> }
>
> <?php
> // my code
> namespace App\Models;
>
> include "something.php";
>
> password_hash('foobar', PASSWORD_DEFAULT);
If this is an attack vector for your application, then fully qualified names is
the way to go (WordPress does this nearly everywhere, for example).
>
> I don't recall why local namespace first won, but IMO it wasn't a great call
> out the gate for that reason alone. Yes, you can always use `\password_hash`
> instead of `password_hash` , but making the default insecure and slower is
> silly IMO -- and not fixing it because of BC seems like the weaker argument
> here.
>
> John
It's not (at least for me) the BC break. It's being able to override global
functions. There are legitimate use-cases outside of testing. For example,
consider when a global function signature changes. In your library, you have to
check the php version. You can change this 100 times for every single call, or
you can just wrap it in a function that supports the old signature and proxies
it to the new signature. In other words, it provides options that may be better
than the alternative.
— Rob
