strutils.mask_dict_password should accept collections.Mappings

Bug #1804528 reported by John Dennis on 2018-11-21
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ben Nemec

Bug Description

This test at the head of mask_dict_password:

    if not isinstance(dictionary, dict):
        raise TypeError("Expected a dictionary, got %s instead."
                        % type(dictionary))

Is too restrictive, it should accept any mapping object. It is not necessary for the dictionary to be mutable (unlike a dict) because mask_dict_password() copies the key/value pairs instead of modifying them. Thus the test should be:

    if not isinstance(dictionary, collections.Mapping):

Note, both of these statements below return True

isinstance({}, collections.Mapping) => True
isinstance({}, collections.MutableMapping) => True

Ben Nemec (bnemec) on 2018-11-28
Changed in oslo.utils:
status: New → Triaged
importance: Undecided → Medium

Fix proposed to branch: master

Changed in oslo.utils:
assignee: nobody → Ben Nemec (bnemec)
status: Triaged → In Progress

Submitter: Zuul
Branch: master

commit ddc436925887b6ece4aba19a36e53ede0b22ae21
Author: Ben Nemec <email address hidden>
Date: Wed Nov 28 19:38:48 2018 +0000

    Support non-dict mappings in mask_dict_password

    mask_dict_password doesn't actually have a dependency on the dict
    type specifically. It can work on any subclass of collections.Mapping.
    This changes the isinstance check to reflect that and adds a unit
    test using a collections.Mapping subclass.

    Change-Id: I28781acf027b9b34f8274196db5dd4d2a9adc9ba
    Closes-Bug: 1804528

Changed in oslo.utils:
status: In Progress → Fix Released

This issue was fixed in the openstack/oslo.utils 3.40.0 release.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers