Add user input boundary test to text ui Timezone settings.
Added label: master.
From: Jiri Konecny jkonecny@redhat.com
Add user input boundary test to text ui Timezone settings. --- pyanaconda/ui/tui/spokes/time_spoke.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/pyanaconda/ui/tui/spokes/time_spoke.py b/pyanaconda/ui/tui/spokes/time_spoke.py index 653df0f..bcb707e 100644 --- a/pyanaconda/ui/tui/spokes/time_spoke.py +++ b/pyanaconda/ui/tui/spokes/time_spoke.py @@ -119,6 +119,8 @@ def input(self, args, key): self._selection = "%s/%s" % (args, self._timezones[args][keyid]) self.apply() self.close() + elif keyid >= len(self._regions) or keyid < 0: + return key else: if len(self._timezones[self._regions[keyid]]) == 1: self._selection = "%s/%s" % (self._regions[keyid],
Looks good to me. We just may need to add the ``< 0`` check in some other places too.
Added label: ACK.
I'm puzzled by a couple of things: * why check ```keyid < 0``` since a negative index is valid in Python? * why is an ```IndexError``` accessing in ```_regions``` array considered a normal behavior...i.e. why is the int key that is out of array bounds still useful to input()'s caller * what about the access that uses keyid in ```self._timezones[args][keyid]``` expression? why is it not necessary to worry about ```IndexError``` in that case?
Removed label: ACK.
why check keyid < 0 since a negative index is valid in Python?
Yes it's valid but it's confusing for the user. I think it's better to set nothing than set something confusing.
why is an IndexError accessing in _regions array considered a normal behavior...i.e. why is the int key that is out of array bounds still useful to input()'s caller
I think this is normal behavior of event processing. There is possible to return ``INPUT_PROCESSED`` instead but I think it's not so clean as this solution. What if someone add some special option later or add default choice to 0 key.
what about the access that uses keyid in self._timezones[args][keyid] expression? why is it not necessary to worry about IndexError in that case?
Yeah that is my fail. I'll fix the pull request because now it's crashing in the inner choice :( . Sorry for that.
anaconda-patches@lists.fedorahosted.org