Skip to content

Fix mixed-content filtering in automatic server selection - #855

Merged
sstidl merged 1 commit into
masterfrom
copilot/fix-librespeed-server-selection
Sep 12, 2026
Merged

sstidl merged 1 commit into
masterfrom
copilot/fix-librespeed-server-selection

Conversation

Copilot AI commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Automatic server selection was rejecting secure backends on http: pages because the protocol check required each server URL to contain location.protocol. This change narrows the filter to the actual mixed-content case: http:// backends on https: pages.

  • Selection logic

    • Replace the protocol substring check in speedtest.js with an explicit mixed-content guard.
    • Keep https:// backends eligible on both http: and https: frontends.
    • Continue skipping insecure http:// backends when the page itself is https:.
  • Regression coverage

    • Add focused coverage for the two protocol combinations that matter:
      • http: frontend + https:// backend is pinged and can be auto-selected
      • https: frontend + http:// backend is still rejected before pinging
  • Behavioral example

    if (
      location.protocol === "https:" &&
      server.server.substring(0, 7).toLowerCase() === "http://"
    ) {
      done();
    }

Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com>
@sstidl
sstidl marked this pull request as ready for review September 12, 2026 14:31
Copilot AI lite review requested due to automatic review settings September 12, 2026 14:31
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix mixed-content filtering during automatic server selection

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Permit HTTPS backends during automatic selection from HTTP frontends.
• Reject HTTP backends only when they would cause HTTPS mixed content.
• Add regression tests for both cross-protocol selection paths.
Diagram

graph TD
  A["Frontend protocol"] --> B["Candidate server"] --> C{"Mixed content?"}
  C -- "Blocked" --> D["Skip backend"]
  C -- "Allowed" --> E["Ping endpoint"] --> F["Select lowest latency"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parse backend URLs with the URL API
  • ➕ Determines protocols through standardized URL parsing.
  • ➕ Handles absolute and protocol-relative URLs through one abstraction.
  • ➖ Introduces broader URL-resolution behavior than this targeted fix requires.
  • ➖ May complicate compatibility with legacy browsers supported by the existing client.

Recommendation: Keep the explicit mixed-content guard. It directly models the browser restriction, preserves protocol-relative backend behavior, minimizes risk in legacy client code, and is adequately protected by focused tests; standardized URL parsing can be considered separately if server URL validation is broadened.

Files changed (2) +118 / -1

Bug fix (1) +5 / -1
speedtest.jsRestrict protocol filtering to actual mixed-content combinations +5/-1

Restrict protocol filtering to actual mixed-content combinations

• Replaces protocol substring matching with an explicit guard that skips HTTP backends only on HTTPS pages. HTTPS backends remain eligible from both HTTP and HTTPS frontends.

speedtest.js

Tests (1) +113 / -0
server-selection.spec.jsAdd cross-protocol server selection regression tests +113/-0

Add cross-protocol server selection regression tests

• Adds a VM-based Speedtest harness with a fake XMLHttpRequest implementation. Tests verify that HTTP frontends can select HTTPS backends while HTTPS frontends reject HTTP backends before pinging them.

tests/e2e/server-selection.spec.js

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved issues were identified.

Pull request overview

Fixes mixed-content filtering so secure backends remain eligible from HTTP pages while insecure backends are blocked on HTTPS pages.

Changes:

  • Replaces protocol matching with an explicit mixed-content guard.
  • Adds regression coverage for both protocol combinations.
File summaries
File Description
tests/e2e/server-selection.spec.js Adds protocol-selection regression coverage.
speedtest.js Corrects server eligibility filtering.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@sstidl
sstidl merged commit be1e0a0 into master Sep 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants