diff --git a/changelogs/fragments/11959-xml-boolean-value.yml b/changelogs/fragments/11959-xml-boolean-value.yml new file mode 100644 index 0000000000..563a21fd22 --- /dev/null +++ b/changelogs/fragments/11959-xml-boolean-value.yml @@ -0,0 +1,4 @@ +bugfixes: + - "xml - emit an error when ``value`` is not a string, pointing to the offending xpath + (https://github.com/ansible-collections/community.general/issues/7171, + https://github.com/ansible-collections/community.general/pull/11959)." diff --git a/plugins/modules/xml.py b/plugins/modules/xml.py index 514ae13a46..65d0ff3411 100644 --- a/plugins/modules/xml.py +++ b/plugins/modules/xml.py @@ -688,6 +688,16 @@ def set_target_inner(module, tree, xpath, namespaces, attribute, value): msg=f"Xpath {xpath} does not reference a node! tree is {etree.tostring(tree, pretty_print=True)}" ) + if not isinstance(value, str): + target = f"attribute '{attribute}' at xpath '{xpath}'" if attribute else f"element text at xpath '{xpath}'" + module.fail_json( + msg=( + f"A non-string value {value!r} was parsed for {target}. " + "YAML values for booleans, octals, floats may not yield the string you intended. " + """Quote the value to be explicit, like `value: "yes"`.""" + ) + ) + for element in tree.xpath(xpath, namespaces=namespaces): if not attribute: changed = changed or (element.text != value) diff --git a/tests/integration/targets/xml/tasks/main.yml b/tests/integration/targets/xml/tasks/main.yml index 620fcb0331..8b613c6a08 100644 --- a/tests/integration/targets/xml/tasks/main.yml +++ b/tests/integration/targets/xml/tasks/main.yml @@ -60,6 +60,7 @@ - include_tasks: test-remove-element.yml - include_tasks: test-remove-element-nochange.yml - include_tasks: test-set-attribute-value.yml + - include_tasks: test-set-attribute-value-boolean.yml - include_tasks: test-set-children-elements.yml - include_tasks: test-set-children-elements-level.yml - include_tasks: test-set-children-elements-value.yml diff --git a/tests/integration/targets/xml/tasks/test-set-attribute-value-boolean.yml b/tests/integration/targets/xml/tasks/test-set-attribute-value-boolean.yml new file mode 100644 index 0000000000..08ec26d20c --- /dev/null +++ b/tests/integration/targets/xml/tasks/test-set-attribute-value-boolean.yml @@ -0,0 +1,59 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +# Regression test for https://github.com/ansible-collections/community.general/issues/7171 +# Passing an unquoted YAML boolean as `value` must now fail with an explicit error. + +- name: Setup test fixture + copy: + src: fixtures/ansible-xml-beers.xml + dest: /tmp/ansible-xml-beers.xml + + +- name: "Set '/business/rating/@subjective' to boolean false (unquoted YAML boolean) - must fail" + xml: + path: /tmp/ansible-xml-beers.xml + xpath: /business/rating + attribute: subjective + value: false + register: set_attribute_boolean_false + ignore_errors: true + +- name: Assert that passing a boolean attribute value raises an error + assert: + that: + - set_attribute_boolean_false is failed + - "'was parsed for' in set_attribute_boolean_false.msg" + - "'attribute' in set_attribute_boolean_false.msg" + + +- name: "Set '/business/rating' text content to boolean true (unquoted YAML boolean) - must fail" + xml: + path: /tmp/ansible-xml-beers.xml + xpath: /business/rating + value: true + register: set_element_boolean_true + ignore_errors: true + +- name: Assert that passing a boolean element value raises an error + assert: + that: + - set_element_boolean_true is failed + - "'was parsed for' in set_element_boolean_true.msg" + - "'element text' in set_element_boolean_true.msg" + + +- name: "Set '/business/rating/@subjective' to quoted string 'false' - must succeed" + xml: + path: /tmp/ansible-xml-beers.xml + xpath: /business/rating + attribute: subjective + value: "false" + register: set_attribute_string_false + +- name: Assert that passing a quoted string attribute value succeeds + assert: + that: + - set_attribute_string_false is changed