From 5cb39d0d57ac5ecbc5f964d6fa32b6cb79edf402 Mon Sep 17 00:00:00 2001 From: Richard Levenberg Date: Tue, 4 Dec 2018 07:19:38 -0800 Subject: [PATCH] Use System.Xml.XmlDocument.Load rather than Get-Content to avoid (#49206) encoding errors when writing out XML removing executable bit refactor tests to handle sanity checking --- .../fragments/48471-win_xml-xml-parser.yaml | 2 + lib/ansible/modules/windows/win_xml.ps1 | 5 +- .../targets/win_xml/files/plane.zip | Bin 0 -> 792 bytes .../targets/win_xml/tasks/main.yml | 63 ++++++++++++++++-- 4 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/48471-win_xml-xml-parser.yaml create mode 100644 test/integration/targets/win_xml/files/plane.zip diff --git a/changelogs/fragments/48471-win_xml-xml-parser.yaml b/changelogs/fragments/48471-win_xml-xml-parser.yaml new file mode 100644 index 0000000000..a455ce9829 --- /dev/null +++ b/changelogs/fragments/48471-win_xml-xml-parser.yaml @@ -0,0 +1,2 @@ +bugfixes: + - win_xml - use New-Object System.Xml.XmlDocument rather than Get-Content for parsing xml (https://github.com/ansible/ansible/issues/48471) diff --git a/lib/ansible/modules/windows/win_xml.ps1 b/lib/ansible/modules/windows/win_xml.ps1 index 97d363fdd6..f722deb614 100644 --- a/lib/ansible/modules/windows/win_xml.ps1 +++ b/lib/ansible/modules/windows/win_xml.ps1 @@ -107,9 +107,10 @@ If (-Not (Test-Path -Path $dest -PathType Leaf)){ Fail-Json $result "Specified path $dest does not exist or is not a file." } -[xml]$xmlorig = $null +$xmlorig = New-Object -TypeName System.Xml.XmlDocument +$xmlorig.XmlResolver = $null Try { - [xml]$xmlorig = Get-Content -Path $dest + $xmlorig.Load($dest) } Catch { Fail-Json $result "Failed to parse file at '$dest' as an XML document: $($_.Exception.Message)" diff --git a/test/integration/targets/win_xml/files/plane.zip b/test/integration/targets/win_xml/files/plane.zip new file mode 100644 index 0000000000000000000000000000000000000000..8157182aaccca28cef24eaef6bd62ffbcfc6da9f GIT binary patch literal 792 zcmWIWW@Zs#U|`^2u+FUZy|?Xh_huk3f{}qigF%L&ASW>|RkySx&Co07fzj8AfWQeV3Csj`gyU!)9>h0*BM zRLIID^Eb22%2;Apy`uHkhwvL5_8a%6YX^ACPmDWo-ef7$PuZ&zSx+5ZHr*zBm5bq) z;$0<6C3M~IKHO&IY`W*K(7Do?!dd5x?#F&M*PLm6PVg(^+nYa=zp<$w_I{IjcaHU0 zpYRzg%~o$Izwxd;FyrO^y!@l@@0}J8KnawkA#*l^0_7GEt04!91vo?!i*i%*G86Mi z2olvkLtu~yxK{faK4e%IawH{|E85zB!zmYz(@6}jQTa)YylNSXdfj%mJT~3aG<)ff zlmGvpRr&w@P2~oE(KiPAk9-b`#N?ZlF+cp!J4vdQnPHFHOz!+gzvo0|pAJ#eNQ)4O zDoQ&Xkglk;(c_gzSSO=d)TUD*)4D77^m#?B(rvn;7!nwxl+L_4J!+YTUHGnGXZ~Fm zxL0uRW4DsL@8sZKCH>VceZg!^>#UUz&-}BT!@K)`fHxzP95b$zA^{8<2-wyLqKQv8 utdMkrmS)gxLyZE6ZNMbTu&q%PNWpC*5FxniVr2ulnF$C%QLMoP;sF342' xpath: '/config' register: element_add_result @@ -35,7 +35,7 @@ - name: try to add the element that only has a text child node again win_xml: - path: "{{win_output_dir}}\\config.xml" + path: "{{ win_output_dir }}\\config.xml" fragment: '42' xpath: '/config' register: element_add_result_second @@ -48,11 +48,11 @@ - name: copy a test log4j.xml win_copy: src: log4j.xml - dest: "{{win_output_dir}}\\log4j.xml" + dest: "{{ win_output_dir }}\\log4j.xml" - name: change an attribute to fatal logging win_xml: - path: "{{win_output_dir}}\\log4j.xml" + path: "{{ win_output_dir }}\\log4j.xml" xpath: '/log4j:configuration/logger[@name="org.apache.commons.digester"]/level' type: attribute attribute: 'value' @@ -60,7 +60,7 @@ - name: try to change the attribute again win_xml: - path: "{{win_output_dir}}\\log4j.xml" + path: "{{ win_output_dir }}\\log4j.xml" xpath: '/log4j:configuration/logger[@name="org.apache.commons.digester"]/level' type: attribute attribute: 'value' @@ -71,3 +71,54 @@ assert: that: - not attribute_changed_result is changed + +# This testing is for https://github.com/ansible/ansible/issues/48471 +# The issue was that an .xml with no encoding declaration, but a UTF8 BOM +# with some UTF-8 characters was being written out with garbage characters. +# The characters added by win_xml were not UTF-8 characters. + +- name: copy test files (https://github.com/ansible/ansible/issues/48471) + win_copy: + src: plane.zip + dest: "{{ win_output_dir }}\\plane.zip" + +- name: unarchive the test files + win_unzip: + src: "{{ win_output_dir }}\\plane.zip" + dest: "{{ win_output_dir }}\\" + +- name: change a text value in a file with UTF8 BOM and armenian characters in the description + win_xml: + path: "{{ win_output_dir }}\\plane-utf8-bom-armenian-characters.xml" + xpath: '/plane/year' + type: text + fragment: '1988' + +- name: register the sha1 of the new file + win_stat: + path: "{{ win_output_dir }}\\plane-utf8-bom-armenian-characters.xml" + get_checksum: yes + register: sha1_checksum + +- name: verify the checksum + assert: + that: + - sha1_checksum.stat.checksum == 'e3e18c3066e1bfce9a5cf87c81353fa174440944' + +- name: change a text value in a file with UTF-16 BE BOM and Chinese characters in the description + win_xml: + path: "{{ win_output_dir }}\\plane-utf16be-bom-chinese-characters.xml" + xpath: '/plane/year' + type: text + fragment: '1988' + +- name: register the sha1 of the new file + win_stat: + path: "{{ win_output_dir}}\\plane-utf16be-bom-chinese-characters.xml" + get_checksum: yes + register: sha1_checksum + +- name: verify the checksum + assert: + that: + - sha1_checksum.stat.checksum == 'de86f79b409383447cf4cf112b20af8ffffcfdbf'