Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quick fix for #101 #102

Merged
merged 4 commits into from
Sep 15, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions repy.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,15 @@ def main():
### anywhere if this is repurposed...
usage = "USAGE: repy.py [options] resource_file program_to_run.r2py [program args]"
parser = optparse.OptionParser(usage=usage)

# Set optparse to stop parsing arguments on the first non-option arg. We
# need this so that command-line args to the sandboxed Repy program don't
# clash or get confused with args to the sandbox (repy.py) itself.
# See also SeattleTestbed/repy_v2#101 .
# (Per the USAGE string above, the user program name is the first
# non-option argument which causes parsing to stop.)
parser.disable_interspersed_args()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may even add a comment explaining the importance of restoring traditional Unix syntax and what can go wrong if command-line options are consumed by repy.py. Not adding a comment here seems like an easy way for a future developer to miss the importance of this behaviour.

Example comment:
disable_interspersed_args() restores traditional Unix syntax, where option parsing stops with the first non-option argument. disable_interspersed_args() should be used if you have a command processor which runs another command which has options of its own and you want to make sure these options don’t get confused. For example, each command might have a different set of options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point, thanks. Addressed in 6c20115.


add_repy_options(parser)
options, args = parser.parse_args()

Expand Down
63 changes: 63 additions & 0 deletions testsV2/ut_repyv2api_commandlineargs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
"""
Verify that the Repy sandbox parses command-line arguments correctly.
For example,
python repy.py restrictionsfile my_program.r2py --logfile ABCDEF

must *not* result in the sandbox consuming the ``--logfile'' arg and
writing the vessel log to the named file. Instead, the sandboxed
program should see the arg.

We test this by calling repy.py with arguments similar to the above,
and a no-op (empty) RepyV2 program.
If repy.py creates the named logfile, this is an error. If the file is
not created, we take this to mean that the sandboxed program gets the
argument (although we don't check this specifically).

Note: This test overwrites / removes files from the current working dir.
The chosen `program_name` and `logfile_prefix` should be unlikely to
clash with anything you created, but you have been warned.
"""

import sys
import os
import portable_popen
import time


# Create an empty RepyV2 program. Remove any previous file of this name first.
program_name = "noop_program_for_repy_callarg_test.r2py"
try:
os.remove(program_name)
except OSError:
# The file wasn't there. No problem.
pass

noop_prog = open(program_name, "w")
noop_prog.close()


# Call Repy. If repy.py mistakingly consumes the --logfile arg, it
# will create a file named logfile_prefix + ".old" (or .new if .old
# already existed).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the file created in the current directory? Might be helpful to explicitly say that log files are created in the current directory.

Note: The repy.py module (i.e., usage description for --logfile on the command-line) doesn't mention the .old and .new extensions : https://github.com/SeattleTestbed/repy_v1/blob/master/repy.py#L34

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be dealt with separately, see #104.

logfile_prefix = "REPY_CALLARG_TEST"
repy_process = portable_popen.Popen([sys.executable, "repy.py",
"restrictions.default", program_name, "--logfile", logfile_prefix])

# Give things time to settle (launching of subprocess, code safety check, etc.)
time.sleep(5)

# See if the logfile was created.
for filename in os.listdir("."):
if filename.startswith(logfile_prefix):
print "Found file", filename, "which indicates that repy.py consumed "
print "the argument meant for the sandboxed program. Bad."
break


# Finally, remove any files we might created
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo: we might have created*

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it to Strunk & White to decide on that one :-)

for filename in [program_name, logfile_prefix+".old", logfile_prefix+".new"]:
try:
os.remove(filename)
except OSError:
pass