Fix UTF double encoding in file editor - #2199
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR standardizes character encoding handling across file content operations by implementing strict UTF-8 validation in the editor, specifying UTF-8 charset in HTTP response headers for file content retrieval, and adding content-type headers to daemon file write requests. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 4❌ Failed checks (4 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This reverts commit 0c0803d.
lancepioch
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Http/Controllers/Api/Client/Servers/FileController.php (1)
69-81:⚠️ Potential issue | 🟡 MinorResponse declares UTF-8 but content is not validated/converted.
EditFiles.phpvalidates and converts content to UTF-8 (Lines 152-156), but this API endpoint serves raw daemon output with acharset=utf-8header. If the file contains non-UTF-8 bytes, the header will be a lie, potentially causing client-side decoding errors.Consider applying the same encoding logic here:
$response = $this->fileRepository->setServer($server)->getContent( $request->get('file'), config('panel.files.max_edit_size') ); + if (!mb_check_encoding($response, 'UTF-8')) { + $response = mb_convert_encoding($response, 'UTF-8', 'ISO-8859-1'); + } + Activity::event('server:file.read') ->property('file', $request->get('file')) ->log();
🧹 Nitpick comments (1)
app/Repositories/Daemon/DaemonFileRepository.php (1)
56-56: Consider specifying charset in the Content-Type header for consistency.The API response in
FileController.phpusestext/plain; charset=utf-8, but the outgoing request here only setstext/plain. For end-to-end consistency in encoding signaling, considertext/plain; charset=utf-8.- ->withBody($content, 'text/plain') + ->withBody($content, 'text/plain; charset=utf-8')
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2187