-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix possible crash due to string conversion error #5018
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds exception handling for potential character encoding conversion errors in network message processing. The changes prevent server crashes when invalid characters are provided in network messages by catching boost::locale::conv::conversion_error exceptions.
Key Changes:
- Added try-catch blocks around
boost::locale::conv::to_utfandfrom_utfcalls to handle conversion errors gracefully - Functions now return empty values instead of crashing when encountering invalid character encodings
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
To highlight the I recommend having all
If this sounds good, then create an issue out of this comment and improve on it, as this is better suited for a different PR. |
Changes Proposed
It is possible that local::conv::to_utf/from_utf8 throw an exception if an invalid char is provided, as far as I am aware a network message can contain user[?] provided messages and thus can crash the server.
It is probably a good idea to use std::expected or std::optional for these error cases in such paths for better error handling, but this is for a different PR.