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

[Xon]

Affected version

2.1.2

Spoiler: stack dump

• 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.

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;
}

 

[Xon]

Adding the transaction doesn't completely stop it, so I'm baffled.

 

[Xon]

MySQL ON DUPLICATE KEY - last insert id?

I have the following query: INSERT INTO table (a) VALUES (0) ON DUPLICATE KEY UPDATE a=1 I want the ID of either the insert or the update. Usually I run a second query in order to get this as I

stackoverflow.com

MySQL :: MySQL 8.0 Reference Manual :: 15.2.7.2 INSERT ... ON DUPLICATE KEY UPDATE Statement

dev.mysql.com

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;

                $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]

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.