-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Validate short and long names so whitespace or empty names cannot be used #6993
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
Validate short and long names so whitespace or empty names cannot be used #6993
Conversation
…hort_name. Firmware should expect at least 1 non-whitespace character for both long_name and short_name. added the USERPREFS_CONFIG_DEVICE_ROLE example to userPrefs.jsonc
…an use whitespace characters. Return BAD_REQUEST error responses when validation fails and warning logs when validation rejects invalid names.
…d to match python cli command
Forgot to comment, PR for python cli made as well: |
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 validation to reject empty or whitespace-only owner and ham names and updates the userPrefs.jsonc
example to include a DEVICE_ROLE
.
- Added
USERPREFS_CONFIG_DEVICE_ROLE
example inuserPrefs.jsonc
- Implemented whitespace/empty-string validation for
long_name
andshort_name
inAdminModule.cpp
- Added similar validation for ham mode parameters in
handleSetHamMode
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
userPrefs.jsonc | Added example for USERPREFS_CONFIG_DEVICE_ROLE |
src/modules/AdminModule.cpp | Enforced non-whitespace name validation for owner and ham modes |
Comments suppressed due to low confidence (2)
src/modules/AdminModule.cpp:158
- Add tests for owner long_name and short_name validation covering empty and whitespace-only inputs to verify the new checks.
case meshtastic_AdminMessage_set_owner_tag:
src/modules/AdminModule.cpp:1179
- Introduce tests for ham call_sign and short_name validation to ensure whitespace-only values are correctly rejected.
// Validate ham parameters before setting since this would bypass validation in the owner struct
if (*r->set_owner.long_name) { | ||
const char *start = r->set_owner.long_name; | ||
// Skip all whitespace (space, tab, newline, etc) | ||
while (*start && isspace((unsigned char)*start)) | ||
start++; | ||
if (*start == '\0') { | ||
LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); | ||
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | ||
break; | ||
} | ||
} | ||
if (*r->set_owner.short_name) { | ||
const char *start = r->set_owner.short_name; | ||
while (*start && isspace((unsigned char)*start)) | ||
start++; | ||
if (*start == '\0') { | ||
LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); | ||
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | ||
break; | ||
} |
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.
Empty owner.long_name values (first character '\0') bypass validation due to the initial if
guard. Remove or adjust the if (*...)
check so that empty strings are also rejected.
if (*r->set_owner.long_name) { | |
const char *start = r->set_owner.long_name; | |
// Skip all whitespace (space, tab, newline, etc) | |
while (*start && isspace((unsigned char)*start)) | |
start++; | |
if (*start == '\0') { | |
LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); | |
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | |
break; | |
} | |
} | |
if (*r->set_owner.short_name) { | |
const char *start = r->set_owner.short_name; | |
while (*start && isspace((unsigned char)*start)) | |
start++; | |
if (*start == '\0') { | |
LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); | |
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | |
break; | |
} | |
const char *start = r->set_owner.long_name; | |
// Skip all whitespace (space, tab, newline, etc) | |
while (*start && isspace((unsigned char)*start)) | |
start++; | |
if (*start == '\0') { | |
LOG_WARN("Rejected long_name: must contain at least 1 non-whitespace character"); | |
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | |
break; | |
} | |
const char *start = r->set_owner.short_name; | |
while (*start && isspace((unsigned char)*start)) | |
start++; | |
if (*start == '\0') { | |
LOG_WARN("Rejected short_name: must contain at least 1 non-whitespace character"); | |
myReply = allocErrorResponse(meshtastic_Routing_Error_BAD_REQUEST, &mp); | |
break; |
Copilot uses AI. Check for mistakes.
I saw feature request #6867 and figured this would be a good addition to the firmware and Python CLI.
I have added validation for empty and whitespace characters as short, long, and ham names. When trying to set them via the CLI, I see the following below in the serial output logs.
Should these be ERROR instead of WARN? I have also modified the Python CLI and will create a PR on that so that you get an error when trying to set blank or whitespace names.
I have also added userPrefs.jsonc USERPREFS_CONFIG_DEVICE_ROLE example addition for #6972 since I forgot to add it in my last PR.
Please let me know if there is anything that needs to be added or modified. I have no idea what I am doing with Android SDK/development, so I believe someone will need to add this validation so the app also gives an error/warning when trying to set these.
🤝 Attestations