fix: Wrong conversion if decimal prefix selected #1626
Conversation
📝 WalkthroughWalkthroughUpdated memoryUsage() in Changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
All contributors have signed the CLA ✍️ ✅ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Filament/Server/Widgets/ServerOverview.php (1)
98-104: Escape clipboard value to prevent JS injection in $this->js()Interpolating raw $value inside a quoted JS string can break the script or enable XSS if the value contains quotes or JS-significant characters (e.g., a malicious server name). Use json_encode to safely embed the value.
Apply this diff:
- $this->js("window.navigator.clipboard.writeText('{$value}');"); + $this->js('window.navigator.clipboard.writeText(' . json_encode($value) . ');');Optional: you can also leverage Filament/Livewire’s built-in copy-to-clipboard patterns to avoid manual JS string interpolation altogether.
🧹 Nitpick comments (2)
app/Filament/Server/Widgets/ServerOverview.php (2)
71-71: Correct fix for decimal vs binary units; consider extracting a shared multiplier helperThis resolves the 16 vs 16.79 GB mismatch when decimal prefixes are selected. To avoid duplication and keep disk/memory consistent, extract a single helper that returns bytes per MB based on the config and use it in both memory and disk.
Apply this diff to use a helper on this line:
- $totalMemory = $this->server->memory * (config('panel.use_binary_prefix') ? 2 ** 20 : 1000 * 1000); + $totalMemory = $this->server->memory * $this->bytesPerMegabyte();Add this helper method to the class (outside the shown range):
private function bytesPerMegabyte(): int { // Default to binary to preserve previous behavior if the config key is missing. return config('panel.use_binary_prefix', true) ? 1024 * 1024 : 1000 * 1000; }Would you like me to open a follow-up PR that introduces this helper and uses it across the widget?
87-87: Keep unit conversion consistent between memory and diskTo make unit handling uniform (and reduce chances of drift), use the same helper for disk as proposed for memory.
Apply this diff:
- $totalBytes = $this->server->disk * (config('panel.use_binary_prefix') ? 1024 * 1024 : 1000 * 1000); + $totalBytes = $this->server->disk * $this->bytesPerMegabyte();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/Filament/Server/Widgets/ServerOverview.php(1 hunks)
🔇 Additional comments (1)
app/Filament/Server/Widgets/ServerOverview.php (1)
71-71: No action needed:use_binary_prefixhas a default
Theuse_binary_prefixkey is defined in config/panel.php withenv('PANEL_USE_BINARY_PREFIX', true), ensuringconfig('panel.use_binary_prefix')always returns a boolean.
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
From 2 ** 20 to 1024 * 1024 Co-authored-by: Boy132 <Boy132@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Filament/Server/Widgets/ServerOverview.php (1)
98-98: Escape JS payload when injecting$valueintocopyClickto prevent breakage/XSSInterpolating
$valuedirectly into a JS string can break on quotes and is vulnerable to script injection. Use JSON-safe embedding.Apply this diff:
- $this->js("window.navigator.clipboard.writeText('{$value}');"); + $this->js('window.navigator.clipboard.writeText(' . \Illuminate\Support\Js::from($value) . ');');And add the import near the other
usestatements (outside the selected range):use Illuminate\Support\Js;
🧹 Nitpick comments (1)
app/Filament/Server/Widgets/ServerOverview.php (1)
71-71: DRY the MB→bytes multiplier into a helper to keep memory/disk consistentCentralize the multiplier logic so future changes (e.g., adding KiB/KB granularity) happen in one place and both memory/disk remain in sync.
Apply this diff to use a helper at both call sites:
- $totalMemory = $this->server->memory * (config('panel.use_binary_prefix') ? 1024 * 1024 : 1000 * 1000); + $totalMemory = $this->server->memory * $this->bytesPerMB();- $totalBytes = $this->server->disk * (config('panel.use_binary_prefix') ? 1024 * 1024 : 1000 * 1000); + $totalBytes = $this->server->disk * $this->bytesPerMB();Add this helper method to the class (outside the selected ranges):
private function bytesPerMB(): int { return config('panel.use_binary_prefix') ? 1024 * 1024 : 1000 * 1000; }Also applies to: 87-87
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/Filament/Server/Widgets/ServerOverview.php(1 hunks)
🔇 Additional comments (1)
app/Filament/Server/Widgets/ServerOverview.php (1)
71-71: Correct multiplier selection for decimal vs binary prefixes — fix looks goodThis aligns memory total with the configured prefix policy and matches the existing disk logic. It addresses the reported 16 GB vs 16.79 GB display issue.
3 participants
When using the decimal prefix and setting 16 GB of RAM, the panel displayed 16.79 GB (without overhead) instead of 16 GB (without overhead).
This was due to the using of a binary multiplier (
2^20), which is only correct for MiB/GiB.So I've used the
config('panel.use_binary_prefix')to check what multiplier should be used.This is a visual fix, but I think it's better like that.