Fixed `\XF\Extension::extendClass()` fails to extend repositories which do not end in `Repository` - XF2.3.11 regression

[Xon]

Affected version

2.3.11

Given the repository SV\Threadmarks\Repository\ThreadmarkCategory, and the add-on accesses ThreadmarkCategory::CONSTANT before calling \XF::repository()` any other add-on attempting to extend this class will fail. [ICODE]\XF\Extension::extendClass() will just return the unexpended class.

The cause is extendClass is calling getAliasedClass which is returning unexpected values due to classLikeExists causing the class to be loaded and then aliased.

public function extendClass($class, $fakeBaseClass = null)
{
    $class = ltrim($class, '\\');
    $class = $this->getAliasedClass($class); //

    if (isset($this->extensionMap[$class]))
    {
        return $this->extensionMap[$class];
    }

    if (!$class)
    {
        return $class;
    }

    $extensions = !empty($this->classExtensions[$class]) ? $this->classExtensions[$class] : [];

 

[Xon]

Refined this found to touching the class before \XF::repository() is called, causing the class alias to be created

 

[Chris D]

Can you check if this resolves it for you or if it causes any further issues?

Index: src/XF/Extension.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/XF/Extension.php b/src/XF/Extension.php
--- a/src/XF/Extension.php    (revision 9cdb012b3158f82e7fad8d0f07af8cb573d3732e)
+++ b/src/XF/Extension.php    (date 1782832468151)
@@ -146,8 +146,8 @@
      */
     public function extendClass($class, $fakeBaseClass = null)
     {
-        $class = ltrim($class, '\\');
-        $class = $this->getAliasedClass($class);
+        $originalClass = ltrim($class, '\\');
+        $class = $this->getAliasedClass($originalClass);
 
         if (isset($this->extensionMap[$class]))
         {
@@ -159,7 +159,9 @@
             return $class;
         }
 
-        $extensions = !empty($this->classExtensions[$class]) ? $this->classExtensions[$class] : [];
+        $extensions = !empty($this->classExtensions[$class])
+            ? $this->classExtensions[$class]
+            : ($this->classExtensions[$originalClass] ?? []);
         if (!$extensions)
         {
             $this->extensionMap[$class] = $class;

 

[Xon]

inverseExtensionMap will need updating for when $originalClass/$class don't match on the early exist points

 

[Kirby]

@Chris D
Will there be a patch for 2.3.11 or will the fix for this issue be in 2.3.12?

 

[Xon]

@Chris D that change is not sufficient as the class extension list for $class and $originalClass need to be merged.

This would need to be done as part of rebuildExtensionCache as that is the place which ahs the execution orders which need merging.

 

[Chris D]

@Chris D
Will there be a patch for 2.3.11 or will the fix for this issue be in 2.3.12?

Probably 2.3.12 at this point as a separate release as too many people will have already upgraded for it to be reasonable to expect people to manually patch.

It can be quite quick, however, but it needs to be fixed first, naturally.

 

[Kirby]

Thanks for the feedback.

Would be nice if the final patch / diff could be attached here so we can apply this before fully rolling out 2.3.11 (assuming that 2.3.12 will take some time).

 

[Chris D]

We will release the patch and get people to verify, but the release will follow very shortly after as this is a fairly significant issue.

 

[Kirby]

This also seems to cause errors like

Fatal Error: Declaration of <VendorPrefix>\<AddOnId>\XF\Pub\Controller\Forum::applyForumFilters(XF\Entity\Forum $forum, XF\Finder\Thread $threadFinder, array $filters) must be compatible with XF\Pub\Controller\ForumController::applyForumFilters(XF\Entity\Forum $forum, XF\Finder\ThreadFinder $threadFinder, array $filters)

if an Add-on (due to XF 2.2 compatibility) uses Finder\Thread instead of Finder\ThreadFinder

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.3.12).

Change log:

Merge class extensions for repository classes registered under both their stripped alias and suffixed name (#2418)

There may be a delay before changes are rolled out to the XenForo Community.

 

[Kirby]

The fix for this bug unfortunately seems to cause a new (kind of) regression:

Up to XenForo 2.3.10 code like

class MiscController extends XFCP_Misc

worked just fine, in XenForo 2.3.12 it does not.

While the 2.3.12 behaviour is probably correct / better, this may break existing Add-ons.

 

[smallwheels]

While the 2.3.12 behaviour is probably correct / better, this may break existing Add-ons.

Jeez! Any way how to find out or test if my installed add ons are affected? I avoided installing 2.3.11 due to this and the other bugs invented through it (ironically through a bug fixing release) and decided to wait for the fix with 2.3.12 (wich surprisingly took a couple of weeks to arrive). Now it turns out that 2.3.12 (the bugfix release for the bugfix release) invents yet another issue, that may technically be no bug but again may cause issues with an existing environment. This is really unfortunate.

 

[Chris D]

The fix for this bug unfortunately seems to cause a new (kind of) regression:

Up to XenForo 2.3.10 code like

class MiscController extends XFCP_Misc

worked just fine, in XenForo 2.3.12 it does not.

While the 2.3.12 behaviour is probably correct / better, this may break existing Add-ons.

It's a correctness thing with any add-ons that hit it. The convention has always been that <class name> extends XFCP_<class name> so for them to be not matching would be a bug in the add-on's code anyway. 2.3.12 surfaces it, but it is now more correct.

Customers should report it to an add-on author if they run into anything.

Jeez! Any way how to find out or test if my installed add ons are affected? I avoided installing 2.3.11 due to this and the other bugs invented through it (ironically through a bug fixing release) and decided to wait for the fix with 2.3.12 (wich surprisingly took a couple of weeks to arrive). Now it turns out that 2.3.12 (the bugfix release for the bugfix release) invents yet another issue, that may technically be no bug but again may cause issues with an existing environment. This is really unfortunate.

It was released to all XenForo Cloud customers a little prior to the release yesterday and there have been no reported issues so far. That's not an entirely "all clear" but somewhat reassuring.

If you upgrade and run into an issue, let me know, the fix (in the add-on code) will be ridiculously simple wherein if a developer is slow to release a fix, you can patch it yourself while you await an updated release.

 

[Kirby]

@Chris D
Yeah, this specific error is certainly a bug in the Add-on, but it is only observable with 2.3.12.

That being said, I still seems to me that the fix has side effect besides this one.

Since upgrading to 2.3.12 we are getting errors like

TypeError: <VendorPrefix>\<AddOnId>\<SomeClass>::init(): Argument #1 ($error) must be of type XF\ControllerPlugin\Error, <VendorPrefix>\<AddOnId>\ControllerPlugin\Error given, called in /<xfroot>/src/addons/<VendorPrefix>/<AddOnId>/XF/ControllerPlugin/Error.php on line <line> src/addons/<VendorPrefix>/<AddOId>/<SomeClass>.php:<line>

<VendorPrefix>\<AddOnId>\ControllerPlugin\Error

in this case is an extension of

XF\ControllerPlugin\Error

so this should be fine,.

But when dumping the aliases from

XF\Extension

right before

init

is called it shows


while it should have

XF\ContollerPlugin\Error

as key.

Still investigating why this happens, will get back when tracked down.

Update
This happens due to checking for file existence in

getAliasForClass()

if the XF 2.0-2.2

Error.php

still exists; this did not happen with 2.3.10 and is another way installations may break after the upgrade.


So it might be worth to add some info to the release notes that such errors can happen (and how to fix them).
Could probably also affect 3rd party Add-ons after an Add-on upgrade if the new version changes to the XF 2.3 naming scheme and old files are not removed.

 

[Xon]

One of the XF2.3 upgrade steps tries to cleanup these orphaned files, but this would accidently delete src/addons/XF/hash.json and break that feature.

Orphaned files are something of a support nightmare as the typical XF add-on update process does not remove old files

 

[Kirby]

Another day, another error likely caused by this:

TypeError: SV\StandardLib\XF\Admin\Controller\AddOn::getSvBatchCreatorService(): Return value must be of type XF\Service\AddOnArchive\InstallBatchCreator, SV\StandardLib\XF\Service\AddOnArchive\InstallBatchCreator returned src/addons/SV/StandardLib/XF/Admin/Controller/AddOn.php:64

(Though with a somewhat older version of StandardLib, no orphaned files for StandardLib).

 

[Xon]

@Kirby Try rebuilding an add-on or the master data. It is very possible that the class extension list didn't get refreshed after pruning old files.

This matters because of how the \XF::getClassForAlias() method is used when building the class extension list is built meaning the old class extension list could be out of date.