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

another fix to bobbit's IRC maximum message length #82

Open
wants to merge 2 commits into
base: bobbit-0.2.x
Choose a base branch
from

Conversation

antithalian
Copy link
Contributor

maximum length includes hostmask too, which means that messages were still being cut off despite checking that command + body + CRNL was <= 512 bytes

the hostmask can change at any time due to operator or service actions, so the most robust fix is setting a conservative maximum length rather than trying to keep an accurate idea of what the bot's hostmask is

…messages

previously, we used len(message)/len(command) to determine when to wrap, but that doesn't necessarily correlate to the number of bytes, which is what the IRC spec cares about
IRC messages can be at most 512 bytes, including the sender's hostmask

determining the hostmask 100% accurately before sending each message is hard, so setting a conservative max length makes the most sense
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 52.83%. Comparing base (d5e90bb) to head (50a0d39).
Report is 2 commits behind head on bobbit-0.2.x.

Files Patch % Lines
src/bobbit/protocol/irc.py 33.33% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##           bobbit-0.2.x      #82   +/-   ##
=============================================
  Coverage         52.83%   52.83%           
=============================================
  Files                21       21           
  Lines               810      810           
=============================================
  Hits                428      428           
  Misses              382      382           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@antithalian
Copy link
Contributor Author

antithalian commented Feb 24, 2024

Note to self: possibly a longer term fix to this would be to discover the hostmask length at session start and then add a new handler for hostmask update messages

I think IRC servers send those when the hostmask is changed, so...

@antithalian
Copy link
Contributor Author

Note on the note to self:

You need to request capabilities in order to get change host messages, so bobbit would need to start doing that and handling the case where the capabilities aren't granted

He'd then need a handler for CHGHOST messages that could figure out the new hostmask and update that and then figure out the new maximum message length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants