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.

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

salt/modules/win_pkg.py list_pkgs is broken (encoding issues) #25532

Closed
attiasr opened this issue Jul 19, 2015 · 2 comments
Closed

salt/modules/win_pkg.py list_pkgs is broken (encoding issues) #25532

attiasr opened this issue Jul 19, 2015 · 2 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Execution-Module P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Windows
Milestone

Comments

@attiasr
Copy link
Contributor

attiasr commented Jul 19, 2015

since the removal of _get_msi_software in win_pkg.py (v2014.7.5...v2014.7.6) the pkg module in windows is not working as expected.
I've tried it with 2015.5 and 2015.8 branches with the same results.

The ''if key in name_map'' condition(https://github.com/saltstack/salt/blob/2015.8/salt/modules/win_pkg.py#L238) is not evaluated properly against the _get_reg_software() keys (I think its due to encoding issues) and therefor the keys are not properly replaces with the mappings and packages keeps trying to reinstalled themselves.
to be more specific the name_map loaded from the repo cache (in get_repo_data) loads str and the keys returned from get_reg_software are unicodes so if you have a character like "®" it will fail to find it in the name_map.
the function _get_msi_software was somehow replacing "®" with "r" so the bug wasn't visible...
Adding those following lines before https://github.com/saltstack/salt/blob/2015.8/salt/modules/win_pkg.py#L236 solve the problem:

u_name_map = {}
for k in name_map.keys():
    u_name_map[unicode(k,'utf-8')] = name_map[k]
name_map = u_name_map
@attiasr attiasr changed the title salt/modules/win_pkg.py list_pkgs is broken since 2014.7.6 salt/modules/win_pkg.py list_pkgs is broken (encoding issues) Jul 19, 2015
@jfindlay jfindlay added Execution-Module Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Windows Platform Relates to OS, containers, platform-based utilities like FS, system based apps P3 Priority 3 labels Jul 20, 2015
@jfindlay jfindlay added this to the Approved milestone Jul 20, 2015
@jfindlay
Copy link
Contributor

@attiasr, thanks for the report. Rather than using unicode, which is not python 3 compatible, we can do

import salt.utils
...
u_name_map = {}
for k in name_map.keys():
    u_name_map[salt.utils.sdecode(k)] = name_map[k]
name_map = u_name_map

This will try the system encoding in addition to UTF-8.

cachedout pushed a commit that referenced this issue Jul 20, 2015
@twangboy twangboy self-assigned this Jul 20, 2015
@twangboy twangboy modified the milestones: Be 1, Approved Jul 20, 2015
@attiasr
Copy link
Contributor Author

attiasr commented Jul 21, 2015

@jfindlay, hey I'm sorry but I've double checked again the solution and salt.utils.sdecode() doesn't properly translate the string, it prefers the default encoding (cp1252) which isn't right for strings returned from _get_reg_software() (utf-8).

Maybe we should move utf-8 to the beginning of the encoding array in the get_encoding() function(https://github.com/attiasr/salt/blob/2015.5/salt/utils/__init__.py#L2441)?

Or instead of using salt.utils.sdecode() which is biased toward the system encoding we can just use the decode function?

u_name_map = {}
for k in name_map.keys():
    u_name_map[k.decode('utf-8')] = name_map[k]
name_map = u_name_map

In any case the validation on https://github.com/attiasr/salt/blob/2015.5/salt/utils/__init__.py#L2458 isn't sufficient to determine proper decoding of strings..

cachedout pushed a commit that referenced this issue Jul 21, 2015
use explicit utf-8 decoding (#25532)
@twangboy twangboy added Awesome and removed Awesome labels Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Execution-Module P3 Priority 3 Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Windows
Projects
None yet
Development

No branches or pull requests

3 participants