Skip to content

Commit

Permalink
Merge pull request #54 from wwkimball/bugfix/ruamel-yaml-crash-wipes-…
Browse files Browse the repository at this point in the history
…out-files-on-set

Bugfix/ruamel yaml crash wipes out files on set
  • Loading branch information
wwkimball committed Jun 3, 2020
2 parents e05113b + ddeae5a commit 3bce9c9
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 6 deletions.
8 changes: 8 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
2.3.5:
Bug Fixes:
* Certain YAML constructs trigger AssertionErrors in ruamel.yaml during YAML
data writes. This was causing yaml-set to generate empty files. Until
https://sourceforge.net/p/ruamel-yaml/tickets/351/ is fixed, this patch
will revert the file contents to mitigate data loss under these conditions.
A specific test has been created to detect when the upstream issue is fixed.

2.3.4:
Bug Fixes:
* Minor security patch: Python already makes non-shell subprocess calls safe
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

setuptools.setup(
name="yamlpath",
version="2.3.4",
version="2.3.5",
description="Read and change YAML/Compatible data using powerful, intuitive, command-line friendly syntax",
long_description=long_description,
long_description_content_type="text/markdown",
Expand Down
33 changes: 32 additions & 1 deletion tests/test_commands_yaml_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ def test_replace_backup_file(self, script_runner, tmp_path_factory):
filedat = fhnd.read()
assert filedat == content


def test_nonref_changes(self, script_runner, tmp_path_factory):
yamlin = """---
somestring:
Expand Down Expand Up @@ -468,3 +467,35 @@ def test_nonref_changes(self, script_runner, tmp_path_factory):
with open(yaml_file, 'r') as fhnd:
filedat = fhnd.read()
assert filedat == yamlout

@pytest.mark.xfail(strict=True, reason="https://sourceforge.net/p/ruamel-yaml/tickets/351/")
def test_commented_aliased_parent_hash(self, script_runner, tmp_path_factory):
yamlin = """---
aliases:
- &key_alias hash
*key_alias :
# Comment
key: value
"""
yamlout = """---
aliases:
- &key_alias hash
*key_alias :
# Comment
key: new value
"""
yaml_file = create_temp_yaml_file(tmp_path_factory, yamlin)
result = script_runner.run(
self.command,
"--change=/hash/key",
"--value=new value",
"--backup",
yaml_file
)
assert result.success, result.stderr

with open(yaml_file, 'r') as fhnd:
filedat = fhnd.read()
assert filedat == yamlout
27 changes: 24 additions & 3 deletions yamlpath/commands/yaml_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@
Copyright 2018, 2019, 2020 William W. Kimball, Jr. MBA MSIS
"""
import sys
import tempfile
import argparse
import secrets
import string
from os import remove, access, R_OK
from os.path import isfile, exists
from shutil import copy2
from shutil import copy2, copyfileobj

from yamlpath.func import clone_node, get_yaml_data, get_yaml_editor
from yamlpath import YAMLPath
Expand Down Expand Up @@ -335,8 +336,28 @@ def main():

# Save the changed file
log.verbose("Writing changed data to {}.".format(args.yaml_file))
with open(args.yaml_file, 'w') as yaml_dump:
yaml.dump(yaml_data, yaml_dump)
with tempfile.TemporaryFile() as tmphnd:
with open(args.yaml_file, 'rb') as inhnd:
copyfileobj(inhnd, tmphnd)

with open(args.yaml_file, 'w') as yaml_dump:
try:
yaml.dump(yaml_data, yaml_dump)
except AssertionError as ex:
yaml_dump.close()
tmphnd.seek(0)
with open(args.yaml_file, 'wb') as outhnd:
copyfileobj(tmphnd, outhnd)

# No sense in preserving a backup file with no changes
if args.backup:
remove(backup_file)

log.debug("Assertion error: {}".format(ex))
log.critical((
"Indeterminate assertion error encountered while"
+ " attempting to write updated data to {}. The original"
+ " file content was restored.").format(args.yaml_file), 3)

if __name__ == "__main__":
main() # pragma: no cover
7 changes: 6 additions & 1 deletion yamlpath/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,12 @@ def _get_optional_nodes(
# pylint: disable=locally-disabled,too-many-nested-blocks
if segments and len(segments) > depth:
(segment_type, unstripped_attrs) = yaml_path.unescaped[depth]
stripped_attrs = segments[depth][1]
stripped_attrs: Union[
str,
int,
SearchTerms,
CollectorTerms
] = segments[depth][1]
except_segment = str(unstripped_attrs)

self.logger.debug(
Expand Down

0 comments on commit 3bce9c9

Please sign in to comment.