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

tell: catch missing message before it causes an exception #2264

Merged
merged 2 commits into from
Apr 22, 2022
Merged

Conversation

dgw
Copy link
Member

@dgw dgw commented Mar 6, 2022

Description

This fixes a regression introduced in v7.1.3 by #2162, backported to the 7.1.x branch as 0dec496. (Yep, this one's a mea culpa.)

Previously, the code was sort-of safe due to a strange use of trigger.group(2).lstrip([…]). The new code, using trigger.group(2).split() instead, did not ensure that it actually gets enough values out of the split. I chose instead to make sure enough "words" exist in the command before performing the split.

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

Acknowledgements

Thanks to monaco (@hedho) for reporting this via logs on our IRC channel.

@dgw dgw added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Mar 6, 2022
@dgw dgw added this to the 7.1.9 milestone Mar 6, 2022
@dgw dgw requested a review from a team March 6, 2022 18:36
Can't put nickname commands here because the pytest plugin doesn't
handle them. That'll be a future patch...
@dgw
Copy link
Member Author

dgw commented Mar 6, 2022

Now with lazy tests using @plugin.example. Covering the whole thing seemed like unnecessary work for a bugfix release, and stuff like testing the nickname commands would require either modifying the pytest plugin or creating a whole new suite of command tests in test_module_tell.py. Didn't think either of those was appropriate in a patch intended for backport.

@dgw dgw merged commit 497f97e into master Apr 22, 2022
@dgw dgw deleted the tell-argcheck branch April 22, 2022 22:17
dgw added a commit that referenced this pull request Apr 22, 2022
tell: catch missing message before it causes an exception
dgw added a commit that referenced this pull request Apr 22, 2022
tell: catch missing message before it causes an exception
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants