Skip to content

cast to unsigned char before tolower/toupper/isdigit calls - #1650

Open
Mounika2456 wants to merge 1 commit into
brainboxdotcc:devfrom
Mounika2456:ctype-unsigned-char
Open

cast to unsigned char before tolower/toupper/isdigit calls#1650
Mounika2456 wants to merge 1 commit into
brainboxdotcc:devfrom
Mounika2456:ctype-unsigned-char

Conversation

@Mounika2456

Copy link
Copy Markdown

dpp::lowercase/dpp::uppercase hand the C tolower/toupper straight to std::transform over a std::string, so every element is a char widened to int:

std::transform(s2.begin(), s2.end(), s2.begin(), tolower);

tolower/toupper are only defined for arguments representable as unsigned char or EOF. Where char is signed, any byte >= 0x80 arrives as a negative value, which is out of that contract. These helpers normalize untrusted input: prefix command names in commandhandler/slashcommand, and the request line plus header keys parsed by the interactions http_server. A UTF-8 name like café or a header key with a high byte is enough to reach the call with a negative argument. Depending on the libc that indexes outside the ctype table, and the MSVC debug CRT asserts on it.

Two isdigit(*str.c_str()) calls in cluster/confirmation.cpp (error field-name handling) have the same problem.

The cast to unsigned char at each call site matches the fix already applied to the tolower lambdas in httpsclient.cpp. Valid input is unaffected; bytes >= 0x80 still pass through unchanged. Built against dev and the offline unit tests pass.

Code change checklist

  • I have ensured that all methods and functions are fully documented using doxygen style comments.
  • My code follows the coding style guide.
  • I tested that my change works before raising the PR.
  • I have ensured that I did not break any existing API calls.
  • I have not built my pull request using AI, a static analysis tool or similar without any human oversight.

@netlify

netlify Bot commented Sep 9, 2026

Copy link
Copy Markdown

Deploy Preview for dpp-dev ready!

Name Link
🔨 Latest commit fad1fd3
🔍 Latest deploy log https://app.netlify.com/projects/dpp-dev/deploys/6aa19ffecc0e480008331087
😎 Deploy Preview https://deploy-preview-1650--dpp-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added documentation Improvements or additions to documentation code Improvements or additions to code. labels Sep 9, 2026
@braindigitalis

Copy link
Copy Markdown
Contributor

have you tested this in real world conditions, not in a fabricated shim?

@braindigitalis

Copy link
Copy Markdown
Contributor

Depending on the libc that indexes outside the ctype table, and the MSVC debug CRT asserts on it.

depending how? this feels like an assumption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code Improvements or additions to code. documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants