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.

Sign up for GitHub

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

Jump to bottom

fix #40973 win_package fails to uninstall a package whose registry Un… #49823

Closed
wants to merge 3 commits into from
Closed

fix #40973 win_package fails to uninstall a package whose registry Un… #49823

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
222727b
fix #40973 win_package fails to uninstall a package whose registry Un…
rufinio Dec 12, 2018
cd34364
fix #40973 win_package: uninstall fails with quoted uninstallString
rufinio Dec 13, 2018
6346db5
fix #40973 win_package: uninstall fails with quoted uninstallString
rufinio Dec 13, 2018
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
7 changes: 6 additions & 1 deletion lib/ansible/modules/windows/win_package.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,12 @@ if ($state -eq "absent") {
$cleanup_artifacts += $local_path
} elseif ($program_metadata.location_type -eq [LocationType]::Empty -and $program_metadata.msi -ne $true) {
# TODO validate the uninstall_string to see if there are extra args in there
$local_path = $program_metadata.uninstall_string
$parsed_uninstall_string = @(ParseCommandLine $program_metadata.uninstall_string)
Copy link
Contributor

Choose a reason for hiding this comment

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

ParseCommandLine isn't a cmdlet I'm aware off which means the code isn't actually doing anything. Can't you just do

$local_path = $program_metadata.uninstall_string
if ($local_path.StartsWith('"') -and $local_path.EndsWith('"')) {
    $local_path = $local_path.Substring(1, $local_path.Length - 2)
}

Copy link
Author

Choose a reason for hiding this comment

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

ParseCommandLine was in Ansible.ModuleUtils.CommandUtil.psm1 and thrown away with 190d1ed

ansible/lib/ansible/module_utils/powershell/Ansible.ModuleUtils.CommandUtil.psm1

Lines 182 to 197 in 691ff4b

public static string[] ParseCommandLine(string lpCommandLine)
{
int numArgs;
IntPtr ret = CommandLineToArgvW(lpCommandLine, out numArgs);
if (ret == IntPtr.Zero)
throw new Win32Exception("Error parsing command line");
IntPtr[] strptrs = new IntPtr[numArgs];
Marshal.Copy(ret, strptrs, 0, numArgs);
string[] cmdlineParts = strptrs.Select(s => Marshal.PtrToStringUni(s)).ToArray();
Marshal.FreeHGlobal(ret);
return cmdlineParts;
}

What about values for uninstall_string like
"C:\Program Files\myprog\uninst.exe" Folder="C:\Documents and Settings\All Users\Application Data\myprog"

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh ok I see what you were trying to do. Unfortunately that wouldn't have worked as you are calling the .NET method but you haven't specified the class it was in. You would have technically had to do [Ansible.CommandUtil]::ParseCommandLine($program_metadata.uninstall_string) but the namespace has been changed from that commit you mentioned so it won't work anymore.

The code was changed to better align with the newer C# module util structure we have recently introduced. At the time of the commit I couldn't find it being used anywhere else and we decided to make the change.

Ultimately I think what we should be doing is to not process the uninstall registry value at all and just pass that directly into Run-Command. That way any comments or arguments that are stored by the app are preserved and we can just append our arguments onto the end of the string if they have been set by the user. This would mean some changes further down to not call Argv-ToString but it should still be possible in this case.

if ($parsed_uninstall_string.Length -eq 1) {
$local_path = $parsed_uninstall_string[0]
} else {
$local_path = $program_metadata.uninstall_string
}
} else {
$local_path = $path
}
Expand Down