Sanitize device fields and prevent crash with numeric hostnames #1089
Conversation
This patch improves the resilience of the guess_icon function by sanitizing mac, vendor, and name fields to avoid crashes caused by unexpected data types (e.g., numeric hostnames). Specifically: mac is now cast to a string before being uppercased, with a newly added fallback to "00:00:00:00:00:00" if empty or invalid. vendor is sanitized to a string before lowercasing, still defaulting to "unknown". name is cast to a string before lowercasing, still falling back to "(unknown)" when empty. This change not only resolves the error caused by numeric-only hostnames (which triggered an AttributeError due to calling .lower() on an int), but also proactively prevents similar crashes from malformed or unexpected input in the future. References: Fixes issue netalertx#1088 and also let's me sleep a little easier tonight.
Fixes netalertx#1088 again. turns out that same thing is used twice. same fix applied
|
Just realized that same set of calls is made twice, and in the second one the string that affected me in the first set of calls i caught onto was sanitized in the second. Made me lol, been there done that. Glad to catch this and fix it. Hoping this PR will bring world peace, end world hunger, and bring joy to everyone. Or at least it'll let me use NetAlertX without having to banish a security camera to the junk drawer. Such funny ways we find edge cases. |
|
lol, thanks a lot @slammingprogramming for the PR and chuckle :) |
|
NOOOO IT ISN'T FULLY READY LOL UNMERGE PLS I'LL FIX AND LET YOU KNOW OMGGGGGG |
|
holy crap. i can do that. i didnt realize i could just roll that back like that. Hooray! I maintain now! I will maintain the heck out of this repo and make it 100 billion percent epic. I'm talking like skins, and options, and settings, and sliders! lots of sliders! I love sliders! A tutorial maybe! Perhaps a new features tour for major updates! And much more accurate automations and more automation integrations! Security patches! Loads of stuff! (I have not enough free time yet I lose sleep to code. I yearn for the code farm.) |
|
Okay, I have confirmed that this does fully work and as such will resubmit. My fuller refactoring is not yet stable but this portion is perfectly fine so far. Going to leave it running and see what happens. |
|
Yeah, fully works from what I can see. I'll get the other fixes I did working sooner than later but for now this resolves issue #1088 and closes out the scope of this PR. |
|
thanks a lot 🙏 @slammingprogramming - feel free to open a PR and I will merge it :) |
do I need to open a new one or is the (below, now?) indicating you were able to do it with the original PR? |
2 participants
This patch improves the resilience of the guess_icon function by sanitizing mac, vendor, and name fields to avoid crashes caused by unexpected data types (e.g., numeric hostnames).
Specifically:
mac is now cast to a string before being uppercased, with a newly added fallback to "00:00:00:00:00:00" if empty or invalid.
vendor is sanitized to a string before lowercasing, still defaulting to "unknown".
name is cast to a string before lowercasing, still falling back to "(unknown)" when empty.
This change not only resolves the error caused by numeric-only hostnames (which triggered an AttributeError due to calling .lower() on an int), but also proactively prevents similar crashes from malformed or unexpected input in the future since I saw those other unsanitized calls for .lower() and .upper() sitting RIGHT THERE so I couldn't in good conscience just leave them there.
References: Fixes issue #1088 and also let's me sleep a little easier tonight.