Page MenuHomePhabricator

Mentions break Phriction preview
Closed, ResolvedPublic

Description

Since D21554, editing a Phriction document containing a user mention causes the following exception:

EXCEPTION: (Error) Call to a member function getPHID() on array at [<phabricator>/src/view/phui/PHUITagView.php:203]
arcanist(head=master, ref.master=76a2976fd9a4), phabricator(head=master, ref.master=387d3b4983d7)
  #0 phlog(Error) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
  #1 PhabricatorAjaxRequestExceptionHandler::handleRequestThrowable(AphrontRequest, Error) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
  #2 AphrontApplicationConfiguration::handleThrowable(Error) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
  #3 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:204]
  #4 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:35]

The array in question is created in PhrictionMarkupPreviewController, and its appearance is already anticipated elsewhere, such as in PhrictionRemarkupRule.

Inserting a check in PHUITagView makes the error go away:

diff --git a/src/view/phui/PHUITagView.php b/src/view/phui/PHUITagView.php
index cd6321a852..d945c9825d 100644
--- a/src/view/phui/PHUITagView.php
+++ b/src/view/phui/PHUITagView.php
@@ -199,7 +199,7 @@ final class PHUITagView extends AphrontTagView {
       );
 
       $context_object = $this->getContextObject();
-      if ($context_object) {
+      if ($context_object && !is_array($context_object)) {
         $hovercard_spec['contextPHID'] = $context_object->getPHID();
       }

However, I don't know enough to say whether this is the proper fix.

Event Timeline

epriestley triaged this task as Low priority.

I ran into this while trying to reproduce the issue:

When Apache is configured with MergeSlashes On (see MergeSlashes in Apache Documentation), which is the default behavior, URIs with multiple consecutive slashes are merged into a single slash before they reach the PHP process. For example, a request to /a//b///c//// reaches PHP as a request for /a/b/c/.

This behavior is sensible for serving files from the filesystem, but does not seem to be mandated by any RFC and can be disabled with MergeSlashes Off.

This generally doesn't cause any concrete problems, except that Phriction attempts to preview the root document with the URI /phriction/preview// (with a semantically relevant trailing double slash) right now.

We could test for this during startup and require users to configure MergeSlashes Off, but I think it's probably better to just not rely on this behavior.