Duplicate InvalidArgumentException: Invalid string IP

[Rasmus Vind]

Affected version

2.3.9

Summary
XF\Job\ApprovalQueueProcess crashes with InvalidArgumentException: Invalid string IP when the moderator log entry is created from CLI.

Reproduce

1. Have content (e.g. a post) sitting in the approval queue.
2. A moderator approves it through the UI, which enqueues an ApprovalQueueProcess job rather than acting inline.
3. The job is later picked up by the CLI runner (php cmd.php xf:run-jobs, or the system cron).

What happens
XF\ModeratorLog\AbstractHandler::setupLogEntityActor() sees $actor->user_id == \XF::visitor()->user_id (because ApprovalQueueProcess::run() wraps the action in \XF::asVisitor($asUser, …)) and calls:

$log->ip_address = Ip::stringToBinary(\XF::app()->request()->getIp());

In CLI context $_SERVER['REMOTE_ADDR'] is unset, so Request::getIp() returns an empty string and Ip::stringToBinary('') throws.

Stack trace

  InvalidArgumentException: Invalid string IP: src/XF/Util/Ip.php:17

  #0 src/XF/ModeratorLog/AbstractHandler.php(137): XF\Util\Ip::stringToBinary('')
  #1 src/XF/ModeratorLog/AbstractHandler.php(118): XF\ModeratorLog\AbstractHandler->setupLogEntityActor(...)
  #2 src/XF/ModeratorLog/AbstractHandler.php(92):  XF\ModeratorLog\AbstractHandler->log(...)
  #3 src/XF/ModeratorLog/Logger.php(79):           XF\ModeratorLog\Logger->logChanges('post', ...)
  #4 src/XF/Logger.php(49):                        XF\Logger->logModeratorChanges('post', ...)
  #5 src/XF/Entity/Post.php(626):                  XF\Entity\Post->_postSave()
  #9 src/XF/Service/Post/ApproverService.php(39):  XF\Mvc\Entity\Entity->save()
  #10 src/XF/ApprovalQueue/PostHandler.php(30):    XF\Service\Post\ApproverService->approve()
  #11 src/XF/ApprovalQueue/AbstractHandler.php(202): XF\ApprovalQueue\PostHandler->actionApprove(...)
  #12 src/XF/Job/ApprovalQueueProcess.php(71):     XF\ApprovalQueue\AbstractHandler->performAction('approve', ...)
  #13 src/XF.php(914):                             XF\Job\ApprovalQueueProcess->XF\Job\{closure}()
  #14 src/XF/Job/ApprovalQueueProcess.php(57):     XF::asVisitor(..., Closure)
  #15 src/XF/Job/Manager.php(275):                 XF\Job\ApprovalQueueProcess->run(8)
  …
  #18 src/XF/Cli/Command/RunJobs.php(58):          XF\Job\Manager->runJobEntry(...)

Suggested fix
Skip the IP assignment when there isn't a real request IP available, e.g.:

protected function setupLogEntityActor(ModeratorLog $log, User $actor)
  {
      $log->user_id = $actor->user_id ?? 0;

      if ($actor->user_id == \XF::visitor()->user_id)
      {
          $ip = \XF::app()->request()->getIp();
          if ($ip !== '')
          {
              $log->ip_address = Ip::stringToBinary($ip);
          }
      }
  }

xf_moderator_log.ip_address is already nullable/blob, so leaving it empty for CLI-originated log rows is consistent with other "no request" code paths.

 

[briansol]

maybe set it to localhost/127 ?

 

[Kirby]

The suggested fix would address this specific scenario, but an IP address not being available via CLI could basically happen everywhere.

I am therefore not sure if this would be the best approach, maybe it would be better to change the bevaviour of Ip::stringToBinary to return null|string with an additional optional param bool $throwOnInvalid = true.
This woud keep backwards compatibility but allow new / updated consumer code to get null (and probably pass this downstream as-is without additional checks everywhere it is called).

 

[XF Bug Bot]

Automated triage: Duplicate

Thank you for reporting this issue. This was fixed in XenForo 2.3.11. Moderator actions run outside a web request, such as approval queue jobs processed by the CLI or cron, no longer try to record a missing IP address. Upgrading to 2.3.11 or later should resolve it.

Duplicate of: https://xenforo.com/community/threads/237406/

This is an automated assessment. A member of staff may review and update it.