The type being used for parsing should be uint32, not int32

[Akasurde]

From @wsteinford on Apr 07, 2020 01:45

ISSUE TYPE

Bug Report

ANSIBLE VERSION

ansible 2.9

CONFIGURATION

OS / ENVIRONMENT

Deploying to Windows host

SUMMARY

https://github.com/ansible/ansible/blob/18a2183a0be3896f28b9fc77f90563585916e98c/lib/ansible/modules/windows/win_package.ps1#L65

This should use uint32 because the function MsiOpenPackageW returns uint32:

 $int_rc = [uint32]::Parse($rc) 

It causes a failure if the expected value exceeds 31 bits, though it should be allowed to reach 32 bits.

STEPS TO REPRODUCE

1. Add a task like this:

- name: Run installer
  win_package:
    path: "installer.msi"
    product_id: "{e2b3431b-3f10-4f53-abdd-d3ff3feaa1ad}"
    # This package usually returns 3221225477 (access exception) when it succeeds installing
    expected_return_code: [0, 3010, 3221225477]

EXPECTED RESULTS

The installer runs and returns 3221225477, and Ansible accepts it as OK

ACTUAL RESULTS

Ansible returns the message:

fatal: [hostname]: FAILED! => {"changed": false, "msg": "failed to parse expected return code 3221225477 as an integer", "reboot_required": false}

Copied from original issue: ansible/ansible#68732

[jborean93]

Integer options in Ansible have traditionally always been a signed integer [Int32] so any values that exceed 2147483647 have to be represented as a negative as the leading bit is whether it is a negative or not. In your case this is -1073741819 which you can derive from:

[Int32]("0x{0:X}" -f ([uint32]3221225477))

So technically you should be able to do expected_return_code: [0, 3010, -1073741819] to achieve the behaviour you want.

Unfortunately when testing this with the following playbook I've encounted another issue that stops it from working.

- hosts: '2019'
  gather_facts: no
  tasks:
  - ansible.windows.win_package:
      path: C:\Windows\System32\cmd.exe
      arguments: /c exit 4294967295  # cmd seems to accept a signed and unsigned integer here
      expected_return_code:
      - 0
      - -1
      state: present
      product_id: not here

The problem is that the code that is returning the exit code back to win_package is returning it as a UInt32 so when we check to see if the rc is in the expected_return_code we don't get a match between the signed and unsigned variant. We have a few possible solutions here

• Change win_package to parse expected_return_code as a list of unsigned integers
  • This could be a breaking change if anybody is passing in a negative already but as we can see it doesn't do anything right now
  • It is technically a departure from our normal standard of using signed integers for fields
• Change the Ansible.ModuleUtils.CommandUtil to return the rc as a signed integer
  • Aligns more of our codebase to use signed integers but has the potential to break a module that is relying on this being an unsigned int
  • I don't think anything in our codebase has that expectation but 3rd party modules might
• Do an explicit conversion in win_package to convert the rc from CommandUtil to the signed variant and compare
  • This is the least obtrusive change as it only affects this part of win_package
  • Keeps things as an int32 which seems to be the standard in both Ansible and PowerShell, $LASTEXITCODE is an int32
  • Would probably need to change the rc return value to be a signed number to avoid any confusion

A fourth option could be a combination of the 2 and have the int field in Ansible a bit more dynamically defined. What I mean is that it will still be an Int32 in PowerShell once parsed but it tries to first parse the signed variant -2147483648 to 2147483647 and if it exceeds 2147483647 it will parse it as a UInt32 then convert to Int32 using the same byte value, i.e. 4294967295 becomes -1.

[jborean93]

I ended up going to option 3 as that was the least obtrusive change required. The PR that implements this fix is #47.

[wsteinford]

Thanks for mentioning the possible workaround of using the negative... I had thought of and tried that, and noticed the same problem you noted.

Thanks for preparing the PR!