From 01bbd71b9683a393a6376b82d481e39884d42158 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Mar 2018 05:00:07 -0800 Subject: [PATCH] Separate the "{img ...}" remarkup rule into separate parse and markup phases Summary: Ref T13101. Ref T4190. This rule is currently single-phase but I'd like to check for a valid proxied image in cache already and just emit an `` tag pointing at it if we have one. To support batching these lookups, split the rule into a parse phase (where we extract URIs) and a markup phase (where we build tags). Test Plan: Used `{img ...}` in Remarkup with no apparent behavioral changes. (This change should do nothing on its own.) Maniphest Tasks: T13101, T4190 Differential Revision: https://secure.phabricator.com/D19192 --- .../markup/PhabricatorImageRemarkupRule.php | 67 +++++++++++++++++-- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/src/applications/files/markup/PhabricatorImageRemarkupRule.php b/src/applications/files/markup/PhabricatorImageRemarkupRule.php index 36089dc57e..69e9d6bcf1 100644 --- a/src/applications/files/markup/PhabricatorImageRemarkupRule.php +++ b/src/applications/files/markup/PhabricatorImageRemarkupRule.php @@ -1,6 +1,9 @@ isFlatText($matches[0])) { return $matches[0]; } + $args = array(); $defaults = array( 'uri' => null, @@ -23,9 +27,10 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { 'width' => null, 'height' => null, ); + $trimmed_match = trim($matches[2]); if ($this->isURI($trimmed_match)) { - $args['uri'] = new PhutilURI($trimmed_match); + $args['uri'] = $trimmed_match; } else { $parser = new PhutilSimpleOptions(); $keys = $parser->parse($trimmed_match); @@ -37,16 +42,64 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { } } if ($uri_key) { - $args['uri'] = new PhutilURI($keys[$uri_key]); + $args['uri'] = $keys[$uri_key]; } $args += $keys; } $args += $defaults; - if ($args['uri']) { + if (!strlen($args['uri'])) { + return $matches[0]; + } + + // Make sure this is something that looks roughly like a real URI. We'll + // validate it more carefully before proxying it, but if whatever the user + // has typed isn't even close, just decline to activate the rule behavior. + try { + $uri = new PhutilURI($args['uri']); + + if (!strlen($uri->getProtocol())) { + return $matches[0]; + } + + $args['uri'] = (string)$uri; + } catch (Exception $ex) { + return $matches[0]; + } + + $engine = $this->getEngine(); + $metadata_key = self::KEY_RULE_EXTERNAL_IMAGE; + $metadata = $engine->getTextMetadata($metadata_key, array()); + + $token = $engine->storeText(''); + + $metadata[] = array( + 'token' => $token, + 'args' => $args, + ); + + $engine->setTextMetadata($metadata_key, $metadata); + + return $token; + } + + public function didMarkupText() { + $engine = $this->getEngine(); + $metadata_key = self::KEY_RULE_EXTERNAL_IMAGE; + $images = $engine->getTextMetadata($metadata_key, array()); + $engine->setTextMetadata($metadata_key, array()); + + if (!$images) { + return; + } + + foreach ($images as $image) { + $args = $image['args']; + $src_uri = id(new PhutilURI('/file/imageproxy/')) - ->setQueryParam('uri', (string)$args['uri']); + ->setQueryParam('uri', $args['uri']); + $img = $this->newTag( 'img', array( @@ -55,9 +108,8 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { 'width' => $args['width'], 'height' => $args['height'], )); - return $this->getEngine()->storeText($img); - } else { - return $matches[0]; + + $engine->overwriteStoredText($image['token'], $img); } } @@ -66,4 +118,5 @@ final class PhabricatorImageRemarkupRule extends PhutilRemarkupRule { // If it does, we'll try to treat it like a valid URI return preg_match('~^https?\:\/\/.*\z~i', $uri_string); } + }