diff --git a/src/xsprintf/PhutilQueryString.php b/src/xsprintf/PhutilQueryString.php --- a/src/xsprintf/PhutilQueryString.php +++ b/src/xsprintf/PhutilQueryString.php @@ -4,6 +4,7 @@ private $maskedString; private $unmaskedString; + private $isMasked; public function __construct(PhutilQsprintfInterface $escaper, array $argv) { // Immediately render the query into a static scalar value. @@ -30,13 +31,63 @@ ), $argv); - $unmasked_string = xsprintf( - 'xsprintf_query', - array( - 'escaper' => $escaper, - 'unmasked' => true, - ), - $argv); + // As an optimization, only render the unmasked string if it may differ + // from the masked string. The strings may differ if any argument is a + // "PhutilOpaqueEnvelope", or if any argument is a "PhutilQueryString" + // with different masked and unmasked values. + + // We always render the masked string and only render the unmasked string + // if we believe we need it, so a mistake here will cause us to incorrectly + // execute a query with "********" in it, not incorrectly disclose + // sensitive query parameters. + + $need_unmasked = false; + foreach ($argv as $arg) { + if ($arg instanceof PhutilQueryString) { + if ($arg->isMasked) { + $need_unmasked = true; + break; + } + continue; + } + + if ($arg instanceof PhutilOpaqueEnvelope) { + $need_unmasked = true; + break; + } + + if (is_array($arg)) { + foreach ($arg as $item) { + if ($item instanceof PhutilQueryString) { + if ($item->isMasked) { + $need_unmasked = true; + break; + } + continue; + } + + // If we find an array item which is not a "PhutilQueryString", we + // know this argument can not require us to unmask the string, since + // no valid argument to "qsprintf(...)" may be a list with a mixture + // of query strings and other values. + break; + } + } + } + + if ($need_unmasked) { + $unmasked_string = xsprintf( + 'xsprintf_query', + array( + 'escaper' => $escaper, + 'unmasked' => true, + ), + $argv); + $this->isMasked = true; + } else { + $unmasked_string = $masked_string; + $this->isMasked = false; + } $this->maskedString = $masked_string; $this->unmaskedString = $unmasked_string;