diff --git a/src/xsprintf/__tests__/PhutilcsprintfTestCase.php b/src/xsprintf/__tests__/PhutilcsprintfTestCase.php index e62ec87..8fd0c6e 100644 --- a/src/xsprintf/__tests__/PhutilcsprintfTestCase.php +++ b/src/xsprintf/__tests__/PhutilcsprintfTestCase.php @@ -1,59 +1,72 @@ assertEqual( + true, + ('ab' === (string)csprintf('%R', 'ab'))); + + // For arguments which have any characters which are not safe in some + // context, %R should apply standard escaping. + $this->assertEqual( + false, + ('a b' === (string)csprintf('%R', 'a b'))); + } + public function testPasswords() { // Normal "%s" doesn't do anything special. $command = csprintf('echo %s', 'hunter2trustno1'); $this->assertEqual( true, strpos($command, 'hunter2trustno1') !== false); - // "%P" takes a PhutilOpaqueEnvelope. $caught = null; try { csprintf('echo %P', 'hunter2trustno1'); } catch (Exception $ex) { $caught = $ex; } $this->assertEqual( true, ($caught instanceof Exception)); // "%P" masks the provided value. $command = csprintf('echo %P', new PhutilOpaqueEnvelope('hunter2trustno1')); $this->assertEqual( false, strpos($command, 'hunter2trustno1')); // Executing the command works as expected. list($out) = execx('%C', $command); $this->assertEqual( true, strpos($out, 'hunter2trustno1') !== false); // Escaping should be robust even when used to escape commands which take // other commands. if (!phutil_is_windows()) { list($out) = execx( 'sh -c %s', csprintf( 'sh -c %s', csprintf( 'sh -c %s', csprintf( 'echo %P', new PhutilOpaqueEnvelope('!@#$%^&*()'))))); $this->assertEqual( true, strpos($out, '!@#$%^&*()') !== false); } } } diff --git a/src/xsprintf/csprintf.php b/src/xsprintf/csprintf.php index dabe466..8ec47f7 100644 --- a/src/xsprintf/csprintf.php +++ b/src/xsprintf/csprintf.php @@ -1,106 +1,118 @@ $pos + 1) ? $pattern[$pos + 1] : null; $is_unmasked = !empty($userdata['unmasked']); if ($value instanceof PhutilCommandString) { if ($is_unmasked) { $value = $value->getUnmaskedString(); } else { $value = $value->getMaskedString(); } } switch ($type) { case 'L': // Only '%Ls' is supported. if ($next !== 's') { throw new Exception("Unknown conversion %L{$next}."); } // Remove the L, leaving %s $pattern = substr_replace($pattern, '', $pos, 1); $length = strlen($pattern); $type = 's'; // Check that the value is a non-empty array. if (!is_array($value)) { throw new Exception("Expected an array for %Ls conversion."); } // Convert the list of strings to a single string. $value = implode(' ', array_map('escapeshellarg', $value)); break; + case 'R': + if (!preg_match('(^[a-zA-Z0-9:/@._-]+$)', $value)) { + $value = escapeshellarg($value); + } + $type = 's'; + break; case 's': $value = escapeshellarg($value); $type = 's'; break; case 'P': if (!($value instanceof PhutilOpaqueEnvelope)) { throw new Exception( "Expected PhutilOpaqueEnvelope for %P conversion."); } if ($is_unmasked) { $value = $value->openEnvelope(); } else { $value = 'xxxxx'; } $value = escapeshellarg($value); $type = 's'; break; case 'C': $type = 's'; break; } $pattern[$pos] = $type; }