diff --git a/changes/92730936fe2f60d077b3b1d9db81bb0f.yaml b/changes/92730936fe2f60d077b3b1d9db81bb0f.yaml new file mode 100644 index 0000000000..49d94e8ce1 --- /dev/null +++ b/changes/92730936fe2f60d077b3b1d9db81bb0f.yaml @@ -0,0 +1,8 @@ +--- +desc: Fixed a bug where the ``stats.countby`` Storm command could raise a ``TypeError`` + when using ``--by-name`` to sort a tally which contained a mix of numeric and non-numeric + values. +desc:literal: false +prs: [] +type: bug +... diff --git a/synapse/lib/stormlib/stats.py b/synapse/lib/stormlib/stats.py index 927783ebe9..3842530e32 100644 --- a/synapse/lib/stormlib/stats.py +++ b/synapse/lib/stormlib/stats.py @@ -1,3 +1,4 @@ +import decimal import collections import synapse.exc as s_exc @@ -86,17 +87,20 @@ async def execStormCmd(self, runt, genr): return if byname: - # Try to sort numerically instead of lexicographically - def coerce(indx): - def wrapped(valu): - valu = valu[indx] - try: - return int(valu) - except ValueError: - return valu - return wrapped - - values = list(sorted(counts.items(), key=coerce(0))) + # Sort numeric names numerically, then non-numeric names lexicographically. + def sortkey(item): + try: + huge = s_common.hugenum(item[0]) + except (ValueError, decimal.DecimalException): + return (1, decimal.Decimal(0), item[0]) + + # non-finite values such as nan cannot be ordered + if not huge.is_finite(): + return (1, decimal.Decimal(0), item[0]) + + return (0, huge, '') + + values = list(sorted(counts.items(), key=sortkey)) maxv = max(val[1] for val in values) else: diff --git a/synapse/tests/test_lib_stormlib_stats.py b/synapse/tests/test_lib_stormlib_stats.py index c1b1ce174f..f0729bd8ad 100644 --- a/synapse/tests/test_lib_stormlib_stats.py +++ b/synapse/tests/test_lib_stormlib_stats.py @@ -136,6 +136,44 @@ 13 | 4 | 26.67% | ######################################## '''.strip() +chartunset_byname = ''' +None | 1 | ######################### + 4 | 1 | ######################### + 1 | 2 | ################################################## + 0 | 1 | ######################### +'''.strip() + +chartunset_rev_byname = ''' + 0 | 1 | ######################### + 1 | 2 | ################################################## + 4 | 1 | ######################### +None | 1 | ######################### +'''.strip() + +chartmixed_byname = ''' + nan | 1 | ################################################## + abc | 1 | ################################################## +None | 1 | ################################################## +0xzz | 1 | ################################################## +0x20 | 1 | ################################################## + 10 | 1 | ################################################## + 2 | 1 | ################################################## + 1.5 | 1 | ################################################## + 1 | 1 | ################################################## +'''.strip() + +chartmixed_rev_byname = ''' + 1 | 1 | ################################################## + 1.5 | 1 | ################################################## + 2 | 1 | ################################################## + 10 | 1 | ################################################## +0x20 | 1 | ################################################## +0xzz | 1 | ################################################## +None | 1 | ################################################## + abc | 1 | ################################################## + nan | 1 | ################################################## +'''.strip() + class StatsTest(s_test.SynTest): @@ -230,6 +268,30 @@ async def test_stormlib_stats_countby(self): with self.raises(s_exc.BadArg): self.len(15, await core.nodes('inet:ipv4 | stats.countby ({})')) + # a tally of numeric values where some nodes do not have the property + # set tallies the unset nodes under None. ( SYN-9957 ) + q = '''[ (inet:ipv4=1.2.3.1 :asn=0) (inet:ipv4=1.2.3.2 :asn=1) (inet:ipv4=1.2.3.3 :asn=1) + (inet:ipv4=1.2.3.4) (inet:ipv4=1.2.3.5 :asn=4) +#unset ]''' + self.len(5, await core.nodes(q)) + + msgs = await core.stormlist('inet:ipv4#unset | stats.countby :asn --by-name') + self.stormIsInPrint(chartunset_byname, msgs) + + msgs = await core.stormlist('inet:ipv4#unset | stats.countby :asn --by-name --reverse') + self.stormIsInPrint(chartunset_rev_byname, msgs) + + # numeric names sort numerically and non-numeric names sort + # lexicographically after them. + q = '''[ it:dev:str="1" it:dev:str="1.5" it:dev:str="2" it:dev:str="10" it:dev:str="nan" + it:dev:str="0x20" it:dev:str="0xzz" it:dev:str="None" it:dev:str="abc" ]''' + self.len(9, await core.nodes(q)) + + msgs = await core.stormlist('it:dev:str | stats.countby --by-name') + self.stormIsInPrint(chartmixed_byname, msgs) + + msgs = await core.stormlist('it:dev:str | stats.countby --by-name --reverse') + self.stormIsInPrint(chartmixed_rev_byname, msgs) + async def test_stormlib_stats_tally(self): async with self.getTestCore() as core: