Fixed canStartConversationWith has potential bad check for isPrivacyCheckMet

Xon

Well-known member
Affected version
2.2.3
Code:
    Error: Macro public:message_macros :: user_info() error: Call to a member function isPrivacyCheckMet() on null src/XF/Entity/User.php:934

    Generated by: Zack Frost Feb 10, 2021 at 4:58 PM

Stack trace

#0 [internal function]: XF\Entity\User->canStartConversationWith(Object(SV\SearchImprovements\XF\Entity\User))
#1 src/XF/Template/Templater.php(1166): call_user_func_array(Array, Array)
#2 internal_data/code_cache/templates/l3/s115/public/message_macros.php(95): XF\Template\Templater->method(Object(SV\SearchImprovements\XF\Entity\User), 'canStartConvers...', Array)
#3 src/XF/Template/Templater.php(799): XF\Template\Templater->{closure}(Object(SV\StandardLib\XF\Template\Templater), Array, NULL)
#4 internal_data/code_cache/templates/l3/s115/public/post_macros.php(31): XF\Template\Templater->callMacro('message_macros', 'user_info', Array, Array)
#5 src/XF/Template/Templater.php(962): XF\Template\Templater->{closure}(Object(SV\StandardLib\XF\Template\Templater), Array, Object(XF\Template\ExtensionSet))
#6 src/XF/Template/Templater.php(898): XF\Template\Templater->renderExtensionInternal('user_cell', Array, Array, Object(XF\Template\ExtensionSet))
#7 internal_data/code_cache/templates/l3/s115/public/post_macros.php(167): XF\Template\Templater->renderExtension('user_cell', Array, Object(XF\Template\ExtensionSet))
#8 src/XF/Template/Templater.php(962): XF\Template\Templater->{closure}(Object(SV\StandardLib\XF\Template\Templater), Array, Object(XF\Template\ExtensionSet))
#9 src/XF/Template/Templater.php(898): XF\Template\Templater->renderExtensionInternal('full_body', Array, Array, Object(XF\Template\ExtensionSet))
#10 internal_data/code_cache/templates/l3/s115/public/post_macros.php(224): XF\Template\Templater->renderExtension('full_body', Array, Object(XF\Template\ExtensionSet))
#11 src/XF/Template/Templater.php(799): XF\Template\Templater->{closure}(Object(SV\StandardLib\XF\Template\Templater), Array, Object(XF\Template\ExtensionSet))
#12 internal_data/code_cache/templates/l3/s115/public/thread_view.php(1035): XF\Template\Templater->callMacro('post_macros', 'post', Array, Array)
#13 src/XF/Template/Templater.php(1626): XF\Template\Templater->{closure}(Object(SV\StandardLib\XF\Template\Templater), Array, Object(XF\Template\ExtensionSet))
#14 src/XF/Template/Template.php(24): XF\Template\Templater->renderTemplate('thread_view', Array)
#15 src/XF/Mvc/Renderer/Html.php(48): XF\Template\Template->render()
#16 src/XF/Mvc/Dispatcher.php(458): XF\Mvc\Renderer\Html->renderView('XF:Thread\\View', 'public:thread_v...', Array)
#17 src/XF/Mvc/Dispatcher.php(440): XF\Mvc\Dispatcher->renderView(Object(XF\Mvc\Renderer\Html), Object(XF\Mvc\Reply\View))
#18 src/XF/Mvc/Dispatcher.php(400): XF\Mvc\Dispatcher->renderReply(Object(XF\Mvc\Renderer\Html), Object(XF\Mvc\Reply\View))
#19 src/XF/Mvc/Dispatcher.php(58): XF\Mvc\Dispatcher->render(Object(XF\Mvc\Reply\View), 'html')
#20 src/XF/App.php(2326): XF\Mvc\Dispatcher->run()
#21 src/XF.php(488): XF\App->run()
#22 index.php(20): XF::runApp('XF\\Pub\\App')
#23 {main}

Request state

array(4) {
  ["url"] => string(42) "/threads/heyo-howdy-my-name-is-riki.42052/"
  ["referrer"] => bool(false)
  ["_GET"] => array(1) {
    ["/threads/heyo-howdy-my-name-is-riki_42052/"] => string(0) ""
  }
  ["_POST"] => array(0) {
  }
}

canStartConversationWith calls $user->Privacy->isPrivacyCheckMet despite all other call sites going via $user->isPrivacyCheckMet
PHP:
    public function canStartConversationWith(\XF\Entity\User $user)
    {
        if (!$this->canBypassUserPrivacy() && !$user->canReceiveConversation())
        {
            return false;
        }

        if (!$user->user_id || $user->user_id == $this->user_id)
        {
            return false;
        }

        return (
            $this->canStartConversation()
            && $user->Privacy->isPrivacyCheckMet('allow_send_personal_conversation', $this)
            && $user->user_state != 'disabled'
            && !$user->is_banned
        );
    }

User::isPrivacyCheckMet implies Privacy may be empty/null, and handles this, but canStartConversationWith does not.
PHP:
    public function isPrivacyCheckMet($privacyKey, User $user)
    {
        if (!$this->Privacy)
        {
            return true;
        }

        /** @var UserPrivacy $privacy */
        $privacy = $this->Privacy;
        return $privacy->isPrivacyCheckMet($privacyKey, $user);
    }
 
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.2.4).

Change log:
Prevent an error when checking if a conversation can be started with a user who is unexpectedly missing part of their profile data.
There may be a delay before changes are rolled out to the XenForo Community.
 
It's worth mentioning that while we do generally protect this sort of call, a user missing any of the 1:1 relationship records (profile, options, privacy, authentication) is very much unexpected and not something that should ever happen within XF. We've tended to see it where the user tables have been manually manipulated.
 
It's worth mentioning that while we do generally protect this sort of call, a user missing any of the 1:1 relationship records (profile, options, privacy, authentication) is very much unexpected and not something that should ever happen within XF. We've tended to see it where the user tables have been manually manipulated.
i don't know if it's important, but i haven't touched my user tables ever, so im assuming an addon screwed something up (i reported this to xon, who started this thread). i had to run a lot of mysql repairs after 2.2, im not a db guy tho so i don't know how to go about diagnosing that.
 
While XF2.0+ should never get into this state natively; a sufficiently old install which has upgraded all the way from v1.0.x/v1.1.x probably has a slightly incoherent database. Especially if add-ons were used ages ago and have long since been removed.

I believe there have been bugs related to account merging which have been fixed over the years
 
Top Bottom