Skip to content

Commit e7367b6

Browse files
committed
Fixed a couple potential crashes on opening files
Fixed crashes that occur when attempting to open a file in a non-existent directory or a when the filename is too long. Specifically fixed this when redirecting output to a file and when saving a transcript based on the history. Also added a couple unit tests related to the fixes.
1 parent 7944147 commit e7367b6

File tree

3 files changed

+82
-11
lines changed

3 files changed

+82
-11
lines changed

cmd2/cmd2.py

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,7 +1696,7 @@ def onecmd_plus_hooks(self, line):
16961696
if self.timing:
16971697
self.pfeedback('Elapsed: %s' % str(datetime.datetime.now() - timestart))
16981698
finally:
1699-
if self.allow_redirection:
1699+
if self.allow_redirection and self.redirecting:
17001700
self._restore_output(statement)
17011701
except EmptyStatement:
17021702
pass
@@ -1840,7 +1840,11 @@ def _redirect_output(self, statement):
18401840
# REDIRECTION_APPEND or REDIRECTION_OUTPUT
18411841
if statement.output == constants.REDIRECTION_APPEND:
18421842
mode = 'a'
1843-
sys.stdout = self.stdout = open(os.path.expanduser(statement.output_to), mode)
1843+
try:
1844+
sys.stdout = self.stdout = open(os.path.expanduser(statement.output_to), mode)
1845+
except (FileNotFoundError, OSError) as ex:
1846+
self.perror('Not Redirecting because - {}'.format(ex), traceback_war=False)
1847+
self.redirecting = False
18441848
else:
18451849
# going to a paste buffer
18461850
sys.stdout = self.stdout = tempfile.TemporaryFile(mode="w+")
@@ -2878,16 +2882,19 @@ def _generate_transcript(self, history, transcript_file):
28782882
self.echo = saved_echo
28792883

28802884
# finally, we can write the transcript out to the file
2881-
with open(transcript_file, 'w') as fout:
2882-
fout.write(transcript)
2883-
2884-
# and let the user know what we did
2885-
if len(history) > 1:
2886-
plural = 'commands and their outputs'
2885+
try:
2886+
with open(transcript_file, 'w') as fout:
2887+
fout.write(transcript)
2888+
except (FileNotFoundError, OSError) as ex:
2889+
self.perror('Failed to save transcript: {}'.format(ex), traceback_war=False)
28872890
else:
2888-
plural = 'command and its output'
2889-
msg = '{} {} saved to transcript file {!r}'
2890-
self.pfeedback(msg.format(len(history), plural, transcript_file))
2891+
# and let the user know what we did
2892+
if len(history) > 1:
2893+
plural = 'commands and their outputs'
2894+
else:
2895+
plural = 'command and its output'
2896+
msg = '{} {} saved to transcript file {!r}'
2897+
self.pfeedback(msg.format(len(history), plural, transcript_file))
28912898

28922899
@with_argument_list
28932900
def do_edit(self, arglist):

tests/test_cmd2.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,44 @@ def test_output_redirection(base_app):
615615
finally:
616616
os.remove(filename)
617617

618+
def test_output_redirection_to_nonexistent_directory(base_app):
619+
filename = '~/fakedir/this_does_not_exist.txt'
620+
621+
# Verify that writing to a file in a non-existent directory doesn't work
622+
run_cmd(base_app, 'help > {}'.format(filename))
623+
expected = normalize(BASE_HELP)
624+
with pytest.raises(FileNotFoundError):
625+
with open(filename) as f:
626+
content = normalize(f.read())
627+
assert content == expected
628+
629+
# Verify that appending to a file also works
630+
run_cmd(base_app, 'help history >> {}'.format(filename))
631+
expected = normalize(BASE_HELP + '\n' + HELP_HISTORY)
632+
with pytest.raises(FileNotFoundError):
633+
with open(filename) as f:
634+
content = normalize(f.read())
635+
assert content == expected
636+
637+
def test_output_redirection_to_too_long_filename(base_app):
638+
filename = '~/sdkfhksdjfhkjdshfkjsdhfkjsdhfkjdshfkjdshfkjshdfkhdsfkjhewfuihewiufhweiufhiweufhiuewhiuewhfiuwehfiuewhfiuewhfiuewhfiuewhiuewhfiuewhfiuewfhiuwehewiufhewiuhfiweuhfiuwehfiuewfhiuwehiuewfhiuewhiewuhfiuewhfiuwefhewiuhewiufhewiufhewiufhewiufhewiufhewiufhewiufhewiuhewiufhewiufhewiuheiufhiuewheiwufhewiufheiufheiufhieuwhfewiuhfeiufhiuewfhiuewheiwuhfiuewhfiuewhfeiuwfhewiufhiuewhiuewhfeiuwhfiuwehfuiwehfiuehiuewhfieuwfhieufhiuewhfeiuwfhiuefhueiwhfw'
639+
640+
# Verify that writing to a file in a non-existent directory doesn't work
641+
run_cmd(base_app, 'help > {}'.format(filename))
642+
expected = normalize(BASE_HELP)
643+
with pytest.raises(OSError):
644+
with open(filename) as f:
645+
content = normalize(f.read())
646+
assert content == expected
647+
648+
# Verify that appending to a file also works
649+
run_cmd(base_app, 'help history >> {}'.format(filename))
650+
expected = normalize(BASE_HELP + '\n' + HELP_HISTORY)
651+
with pytest.raises(OSError):
652+
with open(filename) as f:
653+
content = normalize(f.read())
654+
assert content == expected
655+
618656

619657
def test_feedback_to_output_true(base_app):
620658
base_app.feedback_to_output = True

tests/test_transcript.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,32 @@ def test_history_transcript(request, capsys):
154154

155155
assert transcript == expected
156156

157+
def test_history_transcript_bad_filename(request, capsys):
158+
app = CmdLineApp()
159+
app.stdout = StdOut()
160+
run_cmd(app, 'orate this is\na /multiline/\ncommand;\n')
161+
run_cmd(app, 'speak /tmp/file.txt is not a regex')
162+
163+
expected = r"""(Cmd) orate this is
164+
> a /multiline/
165+
> command;
166+
this is a \/multiline\/ command
167+
(Cmd) speak /tmp/file.txt is not a regex
168+
\/tmp\/file.txt is not a regex
169+
"""
170+
171+
# make a tmp file
172+
history_fname = '~/fakedir/this_does_not_exist.txt'
173+
174+
# tell the history command to create a transcript
175+
run_cmd(app, 'history -t "{}"'.format(history_fname))
176+
177+
# read in the transcript created by the history command
178+
with pytest.raises(FileNotFoundError):
179+
with open(history_fname) as f:
180+
transcript = f.read()
181+
assert transcript == expected
182+
157183
@pytest.mark.parametrize('expected, transformed', [
158184
# strings with zero or one slash or with escaped slashes means no regular
159185
# expression present, so the result should just be what re.escape returns.

0 commit comments

Comments
 (0)