Not a bug Weird code comment and process

[truonglv]

Affected version

2.1.0-beta6

Hello there.

Look into the API Attachment controller I see this.

// If the attachment has a temp hash, then it hasn't been associated yet.
// If we have the hash/key that applies, then that should generally be sufficient.
if (!$attachment->temp_hash)
{
   $container = $attachment->Container;
   if (!$container)
   {
      return $this->noPermission();
   }

   $context = $handler->getContext($container);

   if (\XF::isApiCheckingPermissions() && !$handler->canManageAttachments($context, $error))
   {
      return $this->noPermission($error);
   }
}

So i guess it is should be if($attachment->temp_hash) ?

 

[Chris D]

Nope, I'm pretty sure that code is correct as it is.

That is in XF\Api\Controller\Attachment::actionDelete() so it runs when we are sending a DELETE request for a specific attachment.

If the attachment has a temp_hash then it means that it has not yet been associated to any content. If it has not been associated to any content, then we can just delete it without running any checks. If it has been associated then we need to get the containing content/context and perform the necessary permission checks.

That's what this code does.