-
Notifications
You must be signed in to change notification settings - Fork 11
use message system in exception handler #381
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
Conversation
909e261 to
0862c19
Compare
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 enhances error handling by integrating a message system into the exception handler and improving error logging. The changes allow for better user-facing error messages while maintaining detailed logging for debugging purposes.
- Introduces
logThrowableAndMessageUser()method to provide user-friendly error messages with error IDs - Replaces
uniqid()with a newerrorID()method that generates consistent error IDs based on session and exception object - Adds
$_REQUESTdata to error logs for better debugging context
Comments suppressed due to low confidence (1)
resources/lib/UnityHTTPD.php:101
- The $_REQUEST superglobal may contain sensitive data such as passwords, tokens, or other credentials submitted via forms. Logging this data without sanitization could expose sensitive information in log files. Consider filtering out sensitive fields or documenting that this logging should only be enabled in non-production environments.
$output["_REQUEST"] = $_REQUEST;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
resources/lib/UnityHTTPD.php:114
- Accessing xdebug_message property directly may cause a fatal error if Xdebug is not installed or enabled. The property only exists when Xdebug is active. Consider wrapping this in a check for property_exists($error, 'xdebug_message') or using isset($error->xdebug_message) before accessing it.
?\Throwable $error = null,
resources/lib/UnityHTTPD.php:150
- Security concern: logging the entire $_REQUEST superglobal may expose sensitive data such as passwords, API keys, or tokens that users submit via forms. Consider filtering sensitive fields before logging, or document that sensitive parameter names should follow a convention that can be filtered out.
"msg" => $t->getMessage(),
resources/lib/UnityHTTPD.php:150
errorLognow logs the full$_REQUESTarray, which may include sensitive fields such as passwords, tokens, or session identifiers from GET, POST, or cookies, directly into the error log. Anyone with access to the logs (or an attacker who compromises them) would be able to recover these secrets in cleartext. Restrict logging to non-sensitive request metadata or explicitly redact known secret fields instead of recording the entire$_REQUEST.
"msg" => $t->getMessage(),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
New output is used in the event of an uncaught exception,
badRequest(), orforbidden().gracefulDie()is available for the webpages to make good error messages while handling HTTP POSTs.Also adds
$_REQUESTto error log.in POST:
Screen.Recording.2025-12-11.at.1.54.46.PM.mov
in GET (
display_errors):Screen.Recording.2025-12-11.at.1.55.16.PM.mov
in GET (no
display_errors):Screen.Recording.2025-12-11.at.2.01.42.PM.mov