add simple analytics page - #755
Open
eshirvana wants to merge 4 commits into
Open
Conversation
Contributor
Review Summary by QodoAdd comprehensive analytics dashboard with interactive visualizations
WalkthroughsDescription• Add comprehensive analytics dashboard with key metrics display • Implement interactive Chart.js visualizations for speed test data • Parse and display geographic distribution (countries, cities, ISPs) • Extract browser usage statistics from user agent strings • Support multiple database backends (MySQL, PostgreSQL, MSSQL) Diagramflowchart LR
DB["Database<br/>speedtest_users"] -- "Query metrics" --> Analytics["Analytics Page<br/>analytics.php"]
Analytics -- "Parse data" --> DataFuncs["Data Processing<br/>Functions"]
DataFuncs -- "Speed buckets" --> SpeedBuckets["Speed Distribution<br/>Buckets"]
DataFuncs -- "Ping buckets" --> PingBuckets["Ping Distribution<br/>Buckets"]
DataFuncs -- "Browser parsing" --> Browsers["Browser Detection<br/>from User Agent"]
DataFuncs -- "Location parsing" --> Location["Geographic Data<br/>Countries/Cities/ISPs"]
Analytics -- "Render" --> Charts["Chart.js<br/>Visualizations"]
Charts -- "Display" --> Dashboard["Analytics Dashboard<br/>UI"]
File Changes1. results/analytics.php
|
Contributor
Code Review by Qodo
1. Login/logout not audited
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+465
to
+468
| } elseif ($_SESSION['logged'] === true) { | ||
| if ($_GET['op'] === 'logout') { | ||
| $_SESSION['logged'] = false; | ||
| ?><script type="text/javascript">window.location=location.protocol+"//"+location.host+location.pathname;</script><?php |
Contributor
There was a problem hiding this comment.
1. Login/logout not audited 📘 Rule violation ✧ Quality
• The analytics page performs authentication state changes (login and logout) without recording any audit log entry. • This makes it difficult to reconstruct who accessed sensitive telemetry analytics and when, reducing security investigation capability. • The compliance checklist explicitly includes user login events as critical actions that must be logged with context and outcome.
Agent Prompt
## Issue description
Authentication events (login/logout) are critical actions but are not written to an audit trail.
## Issue Context
This page gates access to analytics derived from telemetry. The compliance checklist requires audit logs for critical actions such as user login events, with sufficient context to reconstruct activity.
## Fix Focus Areas
- results/analytics.php[465-468]
- results/analytics.php[844-846]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+2
to
+4
| session_start(); | ||
| error_reporting(0); | ||
|
|
Contributor
There was a problem hiding this comment.
2. error_reporting(0) hides errors 📘 Rule violation ⛯ Reliability
• The page disables PHP error reporting globally, which can mask warnings/notices and reduce the ability to detect and diagnose production issues. • Combined with returning false on failures, this can cause silent/opaque failures rather than actionable handling. • This conflicts with the requirement to handle failure points with meaningful context and monitoring/logging for debugging.
Agent Prompt
## Issue description
The page suppresses PHP errors and swallows exceptions by returning `false` without logging, leading to silent failures and poor diagnosability.
## Issue Context
The analytics page depends on database queries and JSON parsing; failures should be handled with actionable internal logs (without leaking sensitive details to the UI).
## Fix Focus Areas
- results/analytics.php[2-4]
- results/analytics.php[42-136]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+606
to
+608
| // Tests per day chart | ||
| const testsPerDayData = <?= json_encode($analytics['tests_by_day']) ?>; | ||
| new Chart(document.getElementById('testsPerDayChart'), { |
Contributor
There was a problem hiding this comment.
4. Stored xss via json_encode 🐞 Bug ⛨ Security
• Untrusted telemetry fields (e.g., ispinfo-derived city/ISP/country strings, UA, etc.) are embedded into an inline <script> using raw json_encode output. • json_encode does not prevent </script>-based breakouts by default, so an attacker can store malicious strings via telemetry ingestion and execute JS when an admin views analytics. • This is a stored XSS in an authenticated admin context (stats/analytics pages).
Agent Prompt
## Issue description
Untrusted DB-derived strings are embedded into inline JavaScript via `json_encode(...)` without HTML-safe escaping. This can enable stored XSS (e.g., `</script><script>...</script>`) when viewing the analytics dashboard.
## Issue Context
Telemetry ingestion stores client-supplied `ispinfo` and other values without validation and they are later used as labels/keys in chart datasets.
## Fix Focus Areas
- results/analytics.php[606-817]
- results/telemetry.php[7-19]
- results/analytics.php[230-288]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
added 2 commits
February 5, 2026 13:52
|
Nice work on this @eshirvana — I had a similar itch to scratch and ended up building a more feature-heavy version as a standalone plugin: dakotasnapshot/librespeed-analytics Single PHP file drop-in for the
SQLite-only for now — covers the default LibreSpeed deployment. Figured I would share in case it is useful to anyone following this PR. Happy to collaborate if there is interest in merging ideas. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
sstidl
requested changes
Jun 6, 2026
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
eshirvana
commented
Aug 7, 2026
eshirvana
left a comment
eshirvana
left a comment
Author
There was a problem hiding this comment.
added support for all 3 databases
IRainman
reviewed
Aug 8, 2026
Contributor
There was a problem hiding this comment.
Why don't you use CDN for the library?
Author
There was a problem hiding this comment.
To keep all the resources local so it won't need internet connection. For cases that you check local connection speed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
data (top countries, cities, ISPs)
Dark theme:
Light theme:
