Fixed logPendingUnfurl sometimes returns null due to insert on-dupe & lastInsertId

Affected version
2.1.2

Xon

Well-known member
  • ErrorException: [E_NOTICE] Trying to get property 'url' of non-object
  • src/XF/BbCode/ProcessorAction/AutoLink.php:434
  • Generated by: xxxx
  • May 24, 2019 at 11:18 PM
Stack trace
#0 src/XF/BbCode/ProcessorAction/AutoLink.php(434): XF::handlePhpError(8, '[E_NOTICE] Tryi...', '/var/www/html/u...', 434, Array)
#1 src/XF/BbCode/ProcessorAction/AutoLink.php(425): XF\BbCode\ProcessorAction\AutoLink->getUnfurlBbCode('https://www.cnn...')
#2 src/XF/BbCode/ProcessorAction/AutoLink.php(143): XF\BbCode\ProcessorAction\AutoLink->unfurlLinkUrl('https://www.cnn...')
#3 [internal function]: XF\BbCode\ProcessorAction\AutoLink->XF\BbCode\ProcessorAction\{closure}(Array)
#4 src/XF/BbCode/ProcessorAction/AutoLink.php(154): preg_replace_callback('#^(?<=[^a-z0-9@...', Object(Closure), 'https://www.cnn...')
#5 [internal function]: XF\BbCode\ProcessorAction\AutoLink->filterString('https://www.cnn...', Array, Object(XF\BbCode\Processor))
#6 src/XF/BbCode/Processor.php(375): call_user_func(Array, 'https://www.cnn...', Array, Object(XF\BbCode\Processor))
#7 src/XF/BbCode/Processor.php(358): XF\BbCode\Processor->filterString('https://www.cnn...', Array)
#8 src/XF/BbCode/Traverser.php(65): XF\BbCode\Processor->renderString('https://www.cnn...', Array)
#9 src/XF/BbCode/Traverser.php(37): XF\BbCode\Traverser->renderSubTree(Array, Array)
#10 src/XF/BbCode/Traverser.php(20): XF\BbCode\Traverser->renderAst(Array, Object(SV\AdvancedBbCodesPack\XF\BbCode\RuleSet), Array)
#11 src/XF/Service/Message/Preparer.php(160): XF\BbCode\Traverser->render('https://www.cnn...', Object(SV\AdvancedBbCodesPack\XF\BbCode\Parser), Object(SV\AdvancedBbCodesPack\XF\BbCode\RuleSet), Array)
#12 src/XF/Service/Message/Preparer.php(122): XF\Service\Message\Preparer->processMessage('https://www.cnn...')
#13 src/addons/SV/AdvancedBbCodesPack/XF/Service/Message/Preparer.php(29): XF\Service\Message\Preparer->prepare('https://www.cnn...', true)
#14 src/XF/Service/Post/Preparer.php(98): SV\AdvancedBbCodesPack\XF\Service\Message\Preparer->prepare('https://www.cnn...', true)
#15 src/XF/Service/Thread/Creator.php(148): XF\Service\Post\Preparer->setMessage('https://www.cnn...', true, true)
#16 src/XF/Pub/Controller/Forum.php(545): XF\Service\Thread\Creator->setContent('CNN: A Vietnam ...', 'https://www.cnn...')
#17 src/addons/SV/Threadmarks/XF/Pub/Controller/Forum.php(25): XF\Pub\Controller\Forum->setupThreadCreate(Object(SV\ElasticSearchEssentials\XF\Entity\Forum))
#18 src/addons/SV/ThreadReplyBanner/XF/Pub/Controller/Forum.php(19): SV\Threadmarks\XF\Pub\Controller\Forum->setupThreadCreate(Object(SV\ElasticSearchEssentials\XF\Entity\Forum))
#19 src/XF/Pub/Controller/Forum.php(731): SV\ThreadReplyBanner\XF\Pub\Controller\Forum->setupThreadCreate(Object(SV\ElasticSearchEssentials\XF\Entity\Forum))
#20 src/addons/SV/Threadmarks/XF/Pub/Controller/Forum.php(130): XF\Pub\Controller\Forum->actionPostThread(Object(XF\Mvc\ParameterBag))
#21 src/addons/SV/ElasticSearchEssentials/XF/Pub/Controller/Forum.php(55): SV\Threadmarks\XF\Pub\Controller\Forum->actionPostThread(Object(XF\Mvc\ParameterBag))
#22 src/XF/Mvc/Dispatcher.php(321): SV\ElasticSearchEssentials\XF\Pub\Controller\Forum->actionPostThread(Object(XF\Mvc\ParameterBag))
#23 src/XF/Mvc/Dispatcher.php(248): XF\Mvc\Dispatcher->dispatchClass('XF:Forum', 'PostThread', Object(XF\Mvc\RouteMatch), Object(SV\RedisCache\XF\Pub\Controller\Forum), NULL)
#24 src/XF/Mvc/Dispatcher.php(100): XF\Mvc\Dispatcher->dispatchFromMatch(Object(XF\Mvc\RouteMatch), Object(SV\RedisCache\XF\Pub\Controller\Forum), NULL)
#25 src/XF/Mvc/Dispatcher.php(50): XF\Mvc\Dispatcher->dispatchLoop(Object(XF\Mvc\RouteMatch))
#26 src/XF/App.php(2177): XF\Mvc\Dispatcher->run()
#27 src/XF.php(390): XF\App->run()
#28 index.php(20): XF::runApp('XF\\Pub\\App')
#29 {main}

This was caused by a site using a cloud vendor's SQL product.

The problem is the logPendingUnfurl is sometimes returning null, which getUnfurlBbCode does not handle properly. Wrapping the insert & fetch a transaction "fixes" this error, so perhaps the insert/conditional-fetch isn't working 100% as expected.

PHP:
public function logPendingUnfurl($url)
{
   if (!$url || !preg_match('#^https?://#i', $url))
   {
      throw new \InvalidArgumentException('Invalid URL');
   }

$this->db()->beginTransaction();
   $affected = $this->db()->insert('xf_unfurl_result', [
      'url' => $url,
      'url_hash' => md5($url),
      'pending' => 1
   ], false, '
      pending = VALUES(pending)
   ');
   if ($affected == 1)
   {
      $id = $this->db()->lastInsertId();
      $result =$this->em->find('XF:UnfurlResult', $id);
   }
   else
   {
      $result = $this->getUnfurlResultByUrl($url);
   }
$this->db()->commit();
return $result;
}
 
Last edited:

Xon

Well-known member
If a table contains an AUTO_INCREMENT column and INSERT ... UPDATE inserts a row, the LAST_INSERT_ID() function returns the AUTO_INCREMENT value. If the statement updates a row instead, LAST_INSERT_ID() is not meaningful. However, you can work around this by using LAST_INSERT_ID(expr). Suppose that id is the AUTO_INCREMENT column. To make LAST_INSERT_ID() meaningful for updates, insert rows as follows:
The transaction means the insert & just update (but modify row) happens more rarely (pending set to 1 causing the row change ?)

So the fix would be;
PHP:
                $affected = $this->db()->insert('xf_unfurl_result', [
                        'url' => $url,
                        'url_hash' => md5($url),
                        'pending' => 1
                ], false, '
                        result_id = LAST_INSERT_ID(result_id),
                        pending = VALUES(pending)
                ');
Assuming the logic doesn't need fixing for why the pending flag is being changed.
 

XF Bug Bot

XenForo bug fixer bot
Staff member
Thank you for reporting this issue. It has now been resolved and we are aiming to include it in a future XF release (2.1.5).

Change log:
Implement a transaction and also fetch the last insert id on dupe
Any changes made as a result of this issue being resolved may not be rolled out here until later.
 
Top