-
Notifications
You must be signed in to change notification settings - Fork 11
throw JSON errors, ensure file encoding, handle bad data in errorLog #325
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
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 introduces a new utility function jsonEncode()
to standardize JSON encoding across the codebase with consistent error handling and formatting. The changes enforce the use of JSON_THROW_ON_ERROR
and JSON_UNESCAPED_SLASHES
flags by default, replace all instances of json_encode()
with the new utility, and add enforcement mechanisms to prevent future use of the native function.
- Introduces
jsonEncode()
utility function with standardized flags - Replaces all
json_encode()
calls withjsonEncode()
- Enhances file upload handling with encoding conversion support
- Improves error logging to handle JSON encoding failures gracefully
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
resources/lib/utils.php | Adds new jsonEncode() utility function and updates existing usage |
resources/lib/UnityHTTPD.php | Updates error logging and file upload methods to use new utility and handle encoding |
resources/lib/UnityWebhook.php | Replaces json_encode() with \jsonEncode() |
resources/lib/UnityLDAP.php | Replaces json_encode() with \jsonEncode() |
webroot/api/notices/index.php | Replaces json_encode() with jsonEncode() |
test/assert-no-json_encode.bash | Adds test script to enforce jsonEncode() usage |
test/assert-no-assert.bash | Adds test script to enforce ensure() usage |
.pre-commit-config.yaml | Adds pre-commit hooks for enforcement |
CONTRIBUTING.md | Updates guidelines to document new requirements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7049584
to
e729149
Compare
a4d00f1
to
f52e4bc
Compare
f52e4bc
to
5d59b55
Compare
2d42de7
to
3d1c334
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
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
documentation
\jsonEncode
which always enables the flagsJSON_THROW_ON_ERROR
,JSON_UNESCAPED_SLASHES
phpopenldaper
to also use theJSON_THROW_ON_ERROR
flagjson_encode
withjsonEncode
jsonEncode
must be used instead ofjson_encode
UnityHTTPD::getUploadedFileContents
to encode the file with the specified encoding (default utf8) and doUnityHTTPD::badRequest
upon encoding errorUnityHTTPD::errorLog
to handle the situtation where$data
cannot be JSON encoded, so that the other information can still be logged