Encode file contents to utf-8 #1896
Conversation
📝 WalkthroughWalkthroughTwo server-facing handlers now convert retrieved text to UTF-8 using mb_convert_encoding (trying multiple source encodings including UTF-8, UTF-16, ISO-8859-1, Windows-1252, ASCII) before returning: the file editor's default-content provider and the admin view-install-log modal's log output. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Browser UI
participant Page as Filament Page
participant Server as Remote Server
UI->>Page: Request file content / view install log
Page->>Server: call getContent() / getInstallLogs()
Server-->>Page: raw bytes/string (various encodings)
rect rgb(230,245,255)
Note over Page: convert to UTF-8\ntry encodings: UTF-8, UTF-16, ISO-8859-1, Windows-1252, ASCII
Page->>Page: mb_convert_encoding(raw, 'UTF-8', [ 'UTF-8','UTF-16','ISO-8859-1','Windows-1252','ASCII' ])
end
Page-->>UI: UTF-8 encoded string
Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 |
|
Can you also apply this to server install logs ? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
app/Repositories/Daemon/DaemonRepository.php (1)
setServer(24-31)app/Repositories/Daemon/DaemonServerRepository.php (1)
getInstallLogs(146-152)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
174-176: Good fix for non-ASCII file handling.The encoding conversion addresses issue #1606 by ensuring file contents are properly converted to UTF-8. The explicit encoding list covers common Western encodings that typically cause display issues.
If you want to make this more robust for edge cases with unusual encodings, consider detecting the encoding first:
try { $contents = $this->getDaemonFileRepository()->getContent($this->path, config('panel.files.max_edit_size')); - - return mb_convert_encoding($contents, 'UTF-8', ['UTF-8', 'ISO-8859-1', 'Windows-1252', 'ASCII']); + $encoding = mb_detect_encoding($contents, ['UTF-8', 'ISO-8859-1', 'Windows-1252', 'ASCII'], true) ?: 'UTF-8'; + return mb_convert_encoding($contents, 'UTF-8', $encoding); } catch (FileSizeTooLargeException) {However, the current implementation is sufficient for the reported use case.
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
158-160: Consistent encoding handling across the codebase.The encoding conversion for install logs follows the same pattern as in
EditFiles.php, ensuring consistent UTF-8 normalization across different content retrieval paths. This will prevent display issues with non-ASCII characters in server installation logs.As with the file editor, you could optionally improve robustness by detecting encoding first:
try { $logs = $serverRepository->setServer($server)->getInstallLogs(); - - return mb_convert_encoding($logs, 'UTF-8', ['UTF-8', 'ISO-8859-1', 'Windows-1252', 'ASCII']); + $encoding = mb_detect_encoding($logs, ['UTF-8', 'ISO-8859-1', 'Windows-1252', 'ASCII'], true) ?: 'UTF-8'; + return mb_convert_encoding($logs, 'UTF-8', $encoding); } catch (ConnectionException) {The current explicit list approach is adequate for common scenarios and maintains simplicity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)app/Filament/Server/Resources/Files/Pages/EditFiles.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1671
File: lang/en/server/database.php:22-25
Timestamp: 2025-09-06T20:20:48.706Z
Learning: In the pelican-dev/panel project, translation workflows for non-English languages are handled in separate PRs and processes, so missing translations in other locales should be ignored during code reviews.
🧬 Code graph analysis (2)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
app/Repositories/Daemon/DaemonRepository.php (1)
setServer(24-31)app/Repositories/Daemon/DaemonServerRepository.php (1)
getInstallLogs(146-152)
app/Filament/Server/Resources/Files/Pages/EditFiles.php (3)
app/Http/Controllers/Api/Client/Servers/FileController.php (1)
contents(69-81)app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
getDaemonFileRepository(679-686)app/Repositories/Daemon/DaemonFileRepository.php (1)
getContent(23-43)
Closes #1606
ISO-8859-1 file:
UTF-16LE file:
