Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13210649
D21002.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
14 KB
Referenced Files
None
Subscribers
None
D21002.diff
View Options
diff --git a/scripts/phage.php b/scripts/phage.php
deleted file mode 100755
--- a/scripts/phage.php
+++ /dev/null
@@ -1,29 +0,0 @@
-#!/usr/bin/env php
-<?php
-
-require_once dirname(__FILE__).'/__init_script__.php';
-ini_set('memory_limit', -1);
-
-$args = new PhutilArgumentParser($argv);
-$args->parseStandardArguments();
-
-$args->parsePartial(array());
-
-
-// TODO: This is pretty minimal and should be shared with "arc".
-$working_directory = getcwd();
-$working_copy = ArcanistWorkingCopyIdentity::newFromPath($working_directory);
-$config = id(new ArcanistConfigurationManager())
- ->setWorkingCopyIdentity($working_copy);
-
-foreach ($config->getProjectConfig('load') as $load) {
- $load = Filesystem::resolvePath($working_copy->getProjectRoot().'/'.$load);
- phutil_load_library($load);
-}
-
-
-$workflows = id(new PhutilClassMapQuery())
- ->setAncestorClass('PhageWorkflow')
- ->execute();
-$workflows[] = new PhutilHelpArgumentWorkflow();
-$args->parseWorkflows($workflows);
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -301,6 +301,7 @@
'ArcanistMergeConflictLinter' => 'lint/linter/ArcanistMergeConflictLinter.php',
'ArcanistMergeConflictLinterTestCase' => 'lint/linter/__tests__/ArcanistMergeConflictLinterTestCase.php',
'ArcanistMessageRevisionHardpointLoader' => 'loader/ArcanistMessageRevisionHardpointLoader.php',
+ 'ArcanistMissingArgumentTerminatorException' => 'exception/ArcanistMissingArgumentTerminatorException.php',
'ArcanistMissingLinterException' => 'lint/linter/exception/ArcanistMissingLinterException.php',
'ArcanistModifierOrderingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistModifierOrderingXHPASTLinterRule.php',
'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistModifierOrderingXHPASTLinterRuleTestCase.php',
@@ -885,6 +886,7 @@
'phutil_ini_decode' => 'utils/utils.php',
'phutil_is_hiphop_runtime' => 'utils/utils.php',
'phutil_is_natural_list' => 'utils/utils.php',
+ 'phutil_is_noninteractive' => 'utils/utils.php',
'phutil_is_system_locale_available' => 'utils/utf8.php',
'phutil_is_utf8' => 'utils/utf8.php',
'phutil_is_utf8_slowly' => 'utils/utf8.php',
@@ -1240,6 +1242,7 @@
'ArcanistMergeConflictLinter' => 'ArcanistLinter',
'ArcanistMergeConflictLinterTestCase' => 'ArcanistLinterTestCase',
'ArcanistMessageRevisionHardpointLoader' => 'ArcanistHardpointLoader',
+ 'ArcanistMissingArgumentTerminatorException' => 'Exception',
'ArcanistMissingLinterException' => 'Exception',
'ArcanistModifierOrderingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
'ArcanistModifierOrderingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
diff --git a/src/exception/ArcanistMissingArgumentTerminatorException.php b/src/exception/ArcanistMissingArgumentTerminatorException.php
new file mode 100644
--- /dev/null
+++ b/src/exception/ArcanistMissingArgumentTerminatorException.php
@@ -0,0 +1,4 @@
+<?php
+
+final class ArcanistMissingArgumentTerminatorException
+ extends Exception {}
diff --git a/src/parser/argument/PhutilArgumentParser.php b/src/parser/argument/PhutilArgumentParser.php
--- a/src/parser/argument/PhutilArgumentParser.php
+++ b/src/parser/argument/PhutilArgumentParser.php
@@ -78,6 +78,8 @@
private $synopsis;
private $workflows;
private $showHelp;
+ private $requireArgumentTerminator = false;
+ private $sawTerminator = false;
const PARSE_ERROR_CODE = 77;
@@ -129,8 +131,20 @@
$specs = PhutilArgumentSpecification::newSpecsFromList($specs);
$this->mergeSpecs($specs);
- $specs_by_name = mpull($specs, null, 'getName');
- $specs_by_short = mpull($specs, null, 'getShortAlias');
+ // Wildcard arguments have a name like "argv", but we don't want to
+ // parse a corresponding flag like "--argv". Filter them out before
+ // building a list of available flags.
+ $non_wildcard = array();
+ foreach ($specs as $spec_key => $spec) {
+ if ($spec->getWildcard()) {
+ continue;
+ }
+
+ $non_wildcard[$spec_key] = $spec;
+ }
+
+ $specs_by_name = mpull($non_wildcard, null, 'getName');
+ $specs_by_short = mpull($non_wildcard, null, 'getShortAlias');
unset($specs_by_short[null]);
$argv = $this->argv;
@@ -144,6 +158,7 @@
// Non-string argument; pass it through as-is.
} else if ($arg == '--') {
// This indicates "end of flags".
+ $this->sawTerminator = true;
break;
} else if ($arg == '-') {
// This is a normal argument (e.g., stdin).
@@ -174,7 +189,8 @@
$corrections = PhutilArgumentSpellingCorrector::newFlagCorrector()
->correctSpelling($arg, $options);
- if (count($corrections) == 1) {
+ $should_autocorrect = $this->shouldAutocorrect();
+ if (count($corrections) == 1 && $should_autocorrect) {
$corrected = head($corrections);
$this->logMessage(
@@ -206,7 +222,7 @@
if ($param_name === null) {
throw new PhutilArgumentUsageException(
pht(
- "Argument '%s' does not take a parameter.",
+ 'Argument "%s" does not take a parameter.',
"{$pre}{$arg}"));
}
} else {
@@ -218,7 +234,7 @@
} else {
throw new PhutilArgumentUsageException(
pht(
- "Argument '%s' requires a parameter.",
+ 'Argument "%s" requires a parameter.',
"{$pre}{$arg}"));
}
} else {
@@ -230,7 +246,7 @@
if (array_key_exists($spec->getName(), $this->results)) {
throw new PhutilArgumentUsageException(
pht(
- "Argument '%s' was provided twice.",
+ 'Argument "%s" was provided twice.',
"{$pre}{$arg}"));
}
}
@@ -247,7 +263,7 @@
throw new PhutilArgumentUsageException(
pht(
- "Argument '%s' conflicts with argument '%s'%s",
+ 'Argument "%s" conflicts with argument "%s"%s',
"{$pre}{$arg}",
"--{$conflict}",
$reason));
@@ -279,6 +295,7 @@
}
$this->argv = array_values($argv);
+
return $this;
}
@@ -300,10 +317,20 @@
public function parseFull(array $specs) {
$this->parseInternal($specs, true, false);
- if (count($this->argv)) {
- $arg = head($this->argv);
+ // If we have remaining unconsumed arguments other than a single "--",
+ // fail.
+ $argv = $this->filterWildcardArgv($this->argv);
+ if ($argv) {
throw new PhutilArgumentUsageException(
- pht("Unrecognized argument '%s'.", $arg));
+ pht(
+ 'Unrecognized argument "%s".',
+ head($argv)));
+ }
+
+ if ($this->getRequireArgumentTerminator()) {
+ if (!$this->sawTerminator) {
+ throw new ArcanistMissingArgumentTerminatorException();
+ }
}
if ($this->showHelp) {
@@ -408,7 +435,8 @@
$corrected = PhutilArgumentSpellingCorrector::newCommandCorrector()
->correctSpelling($flow, array_keys($this->workflows));
- if (count($corrected) == 1) {
+ $should_autocorrect = $this->shouldAutocorrect();
+ if (count($corrected) == 1 && $should_autocorrect) {
$corrected = head($corrected);
$this->logMessage(
@@ -551,7 +579,7 @@
if ($xprofile) {
if (!function_exists('xhprof_enable')) {
throw new Exception(
- pht("To use '%s', you must install XHProf.", '--xprofile'));
+ pht('To use "--xprofile", you must install XHProf.'));
}
xhprof_enable(0);
@@ -580,7 +608,7 @@
public function getArg($name) {
if (empty($this->specs[$name])) {
throw new PhutilArgumentSpecificationException(
- pht("No specification exists for argument '%s'!", $name));
+ pht('No specification exists for argument "%s"!', $name));
}
if (idx($this->results, $name) !== null) {
@@ -602,6 +630,14 @@
/* -( Command Help )------------------------------------------------------- */
+ public function setRequireArgumentTerminator($require) {
+ $this->requireArgumentTerminator = $require;
+ return $this;
+ }
+
+ public function getRequireArgumentTerminator() {
+ return $this->requireArgumentTerminator;
+ }
public function setSynopsis($synopsis) {
$this->synopsis = $synopsis;
@@ -750,8 +786,8 @@
throw new PhutilArgumentUsageException(
pht(
- "Argument '%s' is unrecognized. Use '%s' to indicate ".
- "the end of flags.",
+ 'Argument "%s" is unrecognized. Use "%s" to indicate '.
+ 'the end of flags.',
$value,
'--'));
}
@@ -778,7 +814,9 @@
if (isset($this->specs[$name])) {
throw new PhutilArgumentSpecificationException(
- pht("Two argument specifications have the same name ('%s').", $name));
+ pht(
+ 'Two argument specifications have the same name ("%s").',
+ $name));
}
$short = $spec->getShortAlias();
@@ -786,7 +824,7 @@
if (isset($short_map[$short])) {
throw new PhutilArgumentSpecificationException(
pht(
- "Two argument specifications have the same short alias ('%s').",
+ 'Two argument specifications have the same short alias ("%s").',
$short));
}
$short_map[$short] = $spec;
@@ -811,13 +849,15 @@
if (empty($this->specs[$conflict])) {
throw new PhutilArgumentSpecificationException(
pht(
- "Argument '%s' conflicts with unspecified argument '%s'.",
+ 'Argument "%s" conflicts with unspecified argument "%s".',
$name,
$conflict));
}
if ($conflict == $name) {
throw new PhutilArgumentSpecificationException(
- pht("Argument '%s' conflicts with itself!", $name));
+ pht(
+ 'Argument "%s" conflicts with itself!',
+ $name));
}
}
}
@@ -925,11 +965,15 @@
"%B\n%s",
$message,
pht(
- 'For details on available commands, run `%s`.',
+ 'For details on available commands, run "%s".',
"{$binary} help"));
}
throw new PhutilArgumentUsageException($message);
}
+ private function shouldAutocorrect() {
+ return !phutil_is_noninteractive();
+ }
+
}
diff --git a/src/runtime/ArcanistRuntime.php b/src/runtime/ArcanistRuntime.php
--- a/src/runtime/ArcanistRuntime.php
+++ b/src/runtime/ArcanistRuntime.php
@@ -68,6 +68,14 @@
$args = id(new PhutilArgumentParser($argv))
->parseStandardArguments();
+ // If we can test whether STDIN is a TTY, and it isn't, require that "--"
+ // appear in the argument list. This is intended to make it very hard to
+ // write unsafe scripts on top of Arcanist.
+
+ if (phutil_is_noninteractive()) {
+ $args->setRequireArgumentTerminator(true);
+ }
+
$is_trace = $args->getArg('trace');
$log->setShowTraceMessages($is_trace);
@@ -138,6 +146,29 @@
try {
return $args->parseWorkflowsFull($phutil_workflows);
+ } catch (ArcanistMissingArgumentTerminatorException $terminator_exception) {
+ $log->writeHint(
+ pht('USAGE'),
+ pht(
+ '"%s" is being run noninteractively, but the argument list is '.
+ 'missing "--" to indicate end of flags.',
+ $toolset->getToolsetKey()));
+
+ $log->writeHint(
+ pht('USAGE'),
+ pht(
+ 'When running noninteractively, you MUST provide "--" to all '.
+ 'commands (even if they take no arguments).'));
+
+ $log->writeHint(
+ pht('USAGE'),
+ tsprintf(
+ '%s <__%s__>',
+ pht('Learn More:'),
+ 'https://phurl.io/u/noninteractive'));
+
+ throw new PhutilArgumentUsageException(
+ pht('Missing required "--" in argument list.'));
} catch (PhutilArgumentUsageException $usage_exception) {
// TODO: This is very, very hacky; we're trying to let errors like
diff --git a/src/toolset/workflow/ArcanistAliasWorkflow.php b/src/toolset/workflow/ArcanistAliasWorkflow.php
--- a/src/toolset/workflow/ArcanistAliasWorkflow.php
+++ b/src/toolset/workflow/ArcanistAliasWorkflow.php
@@ -145,6 +145,8 @@
// TOOLSETS: Check if the user already has an alias for this trigger, and
// prompt them to overwrite it. Needs prompting to work.
+ // TOOLSETS: Don't let users set aliases which don't resolve to anything.
+
$aliases[] = $alias;
$this->writeAliases($aliases);
diff --git a/src/utils/utils.php b/src/utils/utils.php
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -1918,3 +1918,11 @@
function phutil_unescape_uri_path_component($string) {
return rawurldecode($string);
}
+
+function phutil_is_noninteractive() {
+ if (function_exists('posix_isatty') && !posix_isatty(STDIN)) {
+ return true;
+ }
+
+ return false;
+}
diff --git a/src/workflow/ArcanistWeldWorkflow.php b/src/workflow/ArcanistWeldWorkflow.php
--- a/src/workflow/ArcanistWeldWorkflow.php
+++ b/src/workflow/ArcanistWeldWorkflow.php
@@ -87,8 +87,20 @@
$u = rtrim($u, "\r\n");
$v = rtrim($v, "\r\n");
- $u = phutil_utf8v_combined($u);
- $v = phutil_utf8v_combined($v);
+ // If the inputs are UTF8, split glyphs (so two valid UTF8 inputs always
+ // produce a sensible, valid UTF8 output). If they aren't, split bytes.
+
+ if (phutil_is_utf8($u)) {
+ $u = phutil_utf8v_combined($u);
+ } else {
+ $u = str_split($u);
+ }
+
+ if (phutil_is_utf8($v)) {
+ $v = phutil_utf8v_combined($v);
+ } else {
+ $v = str_split($v);
+ }
$len = max(count($u), count($v));
diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -2282,4 +2282,8 @@
return $this->getConfigurationSourceList()->getConfig($key);
}
+ final public function canHandleSignal($signo) {
+ return false;
+ }
+
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, May 18, 5:04 AM (2 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6293601
Default Alt Text
D21002.diff (14 KB)
Attached To
Mode
D21002: Require "--" as an argument terminator when running noninteractively
Attached
Detach File
Event Timeline
Log In to Comment