coretasks: fix bad WHO tracking endless loop

[HumorBaby]

Description

who_reqs, which is used to track WHO requests to the server, had flipped
keys/values and was never getting cleared upon RPL_ENDOFWHO.
Eventually, this would lead to an endless loop while the bot tried to
get an unused "querytype" (randint) in order to track WHO replies.

Now, key/values are in a more useful order (channel --map--> querytype).
Also, the unneccessary loop used to ensure unique values for the query type was
removed. A RPL_WHOREPLY includes the channel name in the response, so
confirming that the querytype for a channel matched is sufficient. The same
querytype could even be used for every channel without issue.

Checklist

• I have read CONTRIBUTING.md
• I can and do license this contribution under the EFLv2
• No issues are reported by make qa (runs make quality and make test)
• I have tested the functionality of the things this change touches

[Exirel]

Nice catch. This should be noted as a successful week of bug hunt!

[HumorBaby]

Sorry, @Exirel, I have to reject your approval.
I added more 🥺…

See commit message for coretask: update proper usage of WHOX querytype.
TLDR; querytype wasn't being used properly. Updated to spec.

Included the update with this PR since it is closely related to the bugfix, and a separate PR would immediately move/remove some of the fix changes. IMO, should be updated eventually, so why not now, eh?
_{Also, I was extra-:rage: because, had it been used correctly to begin, this fix would not have been necessary.}

I'm happy to discuss alternatives to this combo.

PR has changed significantly (4fc75c1) since review

[dgw]

I'm nitpicking. It's my job, no? 😁

Nothing here is part of Sopel's documented API, thankfully. This is probably safe to release as part of 7.1.1, though I wish the "type" of WHO query could be non-numeric. 000 is something other people might already use in their plugins (though I did search GitHub for "sopel who 000" in all indexed Python code and found a total of 10 irrelevant hits—it's probably perfectly fine).

Oh well. Up to now it was possible to get Heisenbugs if the random integer happened to collide with some custom code's WHO requests. Now it'll be consistent, and therefore easier to find/fix.

[HumorBaby]

… though I wish the "type" of WHO query could be non-numeric. 000 …

Wellll actually, nowhere does it "officially" say it has to be numeric (where officially refers to IRCd docs, and not random forums on the interwebz). The pages cited in the comments (and commit message) use numbers in the example and one uses the term "digits" (which I believe hope was meant as characters 🤞)

Looking at the source for IRCd's that implement WHOX (I checked ergo/oregano, InspIRCd, UnderNet's ircu2, Libera.chat's solanum, and freenode's ircd-seven) and they all store querytype as a language-specific string (e.g., char[] what have you). There is no enforcement of numeric querytypes. The only validation+enforment being: if your querytype is longer than three characters, they auto-default to "0" (still string).

… what to do?? "Lmk"?

[dgw]

Yeah, you read the same Faerion page I did, which is where the "digits" thing came from. What annoys me about WHOX is that there seems to be no real spec to follow…which means we're stuck doing what you did, and what I presume the previous maintainers did: digging through IRCd source code. Not great.

In the absence of such a spec, though, I'd be wary of doing anything that might run afoul of older IRCds' logic. Your checks validate that being numeric isn't required now, but perhaps it was in the past. A counterexample I found is from DareNET's current documentation (snapshot) on WHOX: it specifies "value between 0 and 999".

Seriously, though. We can commiserate over the lack of a formal standard for WHOX all we want, but the endgame is always going to be "leave it as 000; it's fine". 😉

[Exirel]

Same as dgw: let's remove that end_who plugin callable.

[HumorBaby]

In the absence of such a spec, though, I'd be wary of doing anything that might run afoul of older IRCds' logic. Your checks validate that being numeric isn't required now, but perhaps it was in the past. A counterexample I found is from DareNET's current documentation (snapshot) on WHOX: it specifies "value between 0 and 999".

Seriously, though. We can commiserate over the lack of a formal standard for WHOX all we want, but the endgame is always going to be "leave it as 000; it's fine".

@dgw, based on your reasoning, I have actually decided to change it to "999". Sopel checks for correct querytype as a string match, despite being numeric. So, if an old IRCd casts a querytype with value < 100 to a numeric, it may end up causing false negatives when, for example, "42" != "042".

In other news, I made the rest of the requested changes and squashed the original loop fix + set querytype constant commits.

[dgw]

🎉🥳🚀

So, if an old IRCd casts a querytype with value < 100 to a numeric, it may end up causing false negatives when, for example, "42" != "042".

That is a good point! It's no reason to amend the PR, but just so you know how silly I am: I probably would have picked something like "Sopel" encoded to T9 with the vowels removed ("775"), or a common "lucky" number like 777 or 888. 😁

[Exirel]

Let's merge that and publish a 7.1.1 asap.