Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve magic constants to a resolved path #69

Open
djschilling opened this issue Nov 7, 2023 · 7 comments
Open

Resolve magic constants to a resolved path #69

djschilling opened this issue Nov 7, 2023 · 7 comments
Assignees
Labels
bug 🐞 Something isn't working

Comments

@djschilling
Copy link

I have some php code that looks like this:

    public function checkInternalGroupPermissionForPerson(string $auth, ?int $personId) {
        include_once dirname(__DIR__, 3) . '/churchdb/churchdb_db.php';
        $groups = getMyGroupIdsWhereIAmAllowed($auth);
        $personIds = churchdb_getAllPeopleIdsFromGroups($groups);

        return isset($personIds[$personId]);
    }

The lives in a class for which php-aop creates a proxy. The file churchdb_db.php does not need a proxy and so the include does not work.
How can i solve this?

@WalterWoshid WalterWoshid self-assigned this Nov 7, 2023
@WalterWoshid WalterWoshid added the bug 🐞 Something isn't working label Nov 7, 2023
@WalterWoshid WalterWoshid changed the title includes relative to current folder are not working Resolve magic constants to a resolved path Nov 7, 2023
@WalterWoshid
Copy link
Contributor

Hi @djschilling,

I will have a look at this in the evening :)

@WalterWoshid
Copy link
Contributor

WalterWoshid commented Nov 7, 2023

@WalterWoshid
Copy link
Contributor

This seems to be a bigger problem than I thought. This is, like I thought, a problem with magic constants. When a class is matched by AOP, it will be proxied and woven. The proxy is a 1:1 copy of the original class which extends and includes the woven class. The woven class is the class that does the whole AOP logic. Inside the proxied class I can replace all magic constants, which works. I will commit this change shortly.

But here is where it gets complicated, lets imagine a matched class has a trait or extends another class. If that class also includes magic constants like __class__ or even self::class, they also have to be resolved, but won't be, because they itself aren't matched, only the class that uses them.

Possible solutions:

Solution 1:

  • If a class matches, proxify all their traits, parent classes and interfaces as well without creating a woven class.

Problems:

  • What if a non-matched class is loaded first and has the same trait/parent class/interface as well? Once the ClassLoader loads a class, it cannot be replaced (at least in the ClassLoader itself, maybe there is a spl_ function that can achieve that?)

Solution 2:

  • Just proxify EVERY class

Problems:

  • Unnecessary matching
  • Can lead to more problems
  • Probably slower

@WalterWoshid
Copy link
Contributor

Hi @djschilling,

I have released Okapi/AOP - 1.2.9 which fixes the bug partially. You should have no problems when using __DIR__ anymore, but when your matched class has a trait/interface/parent class which uses __class__ or self::class and is not matched by any aspect, it might have a __AopProxied suffix. Besides that, everything should work fine now, especially for your use-case :)

I'm keeping this issue open for the other bugs.

@djschilling
Copy link
Author

I updated to 1.2.9 but this is still not working:

    public function getGroupsWhereUserCan($permission, ?array $groupIds = null): array {
        include_once dirname(__DIR__, 3) . '/churchdb/churchdb_db.php';
        return \getMyGroupIdsWhereIAmAllowed($permission, null, $groupIds);
    }

Here is the error:

include_once(/private/var/folders/nx/1shmdfr54sv9r22ql0c4dh680000gn/T/ct-aop-cache/32117/proxies/churchdb/churchdb_db.php): Failed to open stream: No such file or directory

/private/var/folders/nx/1shmdfr54sv9r22ql0c4dh680000gn/T/ct-aop-cache/32117/proxies/src/Auth/Permissions/PermChecker.php:309
/private/var/folders/nx/1shmdfr54sv9r22ql0c4dh680000gn/T/ct-aop-cache/32117/proxies/src/Auth/Permissions/PermChecker.php:309
/Users/david/projects/churchtools/system/src/Domain/Finance/CostCenter/CostCentersAccountingPeriodPermissionResolver.php:40

PermChecker.php : system/src/Auth/Permissions/PermChecker.php
churchdb_db.php: system/churchdb/churchdb_db.php

@WalterWoshid
Copy link
Contributor

Hi @djschilling,

I will try it with your code and see what's going wrong.

@WalterWoshid
Copy link
Contributor

Hi @djschilling,

Sadly I can't reproduce your error.

I tried testing it like this: $this->data = require dirname(__DIR__, 3) . '/AdviceBehavior/Include/Database/data.php'; and it worked without a problem.

The code in the __AopProxied class does look like this: require dirname('C:\Projects\okapi\php-aop\tests\Functional\AdviceBehavior\Include\Target', 3) . '/AdviceBehavior/Include/Database/data.php';

Can you tell me what your proxy file looks like?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants