Page MenuHomePhabricator

Allow certain flags to be required to appear before non-flag arguments
ClosedPublic

Authored by epriestley on Sep 14 2018, 6:50 PM.
Tags
None
Referenced Files
F12860264: D19675.diff
Fri, Mar 29, 1:11 PM
F12847320: D19675.diff
Fri, Mar 29, 2:04 AM
Unknown Object (File)
Wed, Mar 20, 10:26 PM
Unknown Object (File)
Wed, Mar 20, 10:26 PM
Unknown Object (File)
Thu, Mar 14, 4:54 PM
Unknown Object (File)
Thu, Mar 14, 4:53 PM
Unknown Object (File)
Thu, Mar 14, 4:53 PM
Unknown Object (File)
Sun, Mar 10, 8:04 AM
Subscribers
None

Details

Summary

Ref T13098. Historically, we've seen some issues (particularly with Mercurial) where naming a branch something like --execute-remote-website=evil.com would evaluate even with "--" in the argument list (see T13012).

We're broadly better about this, but still need to parse arguments in multiple phases, and callers/users aren't always careful about using -- to terminate argument lists.

To reduce the danger of flags like --config, --load-library, etc., allow argument parsing to require that they appear before workflow arguments, so arc --config xyz diff is valid, but arc diff --config xyz is not. This reduces the chance that some user script somewhere which does arc diff %s instead of arc diff -- %s will ever be able to do anything truly dangerous.

Test Plan

See upcoming changes to Arcanist. (This does nothing on its own.)

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

amckinley added inline comments.
src/parser/argument/PhutilArgumentParser.php
111–114

Just a nitpick, but this spacing is all over the place.

597–600

This is unused code which I'm assuming will get used in a coming revision.

This revision is now accepted and ready to land.Sep 14 2018, 7:51 PM
This revision was automatically updated to reflect the committed changes.