librespeed-cli: update to 1.0.14

All twelve backported patches are part of this release, so dropping them.
Release notes: https://github.com/librespeed/speedtest-cli/releases/tag/v1.0.14

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
pull/30293/head
Josef Schlehofer 2026-08-17 09:12:36 +02:00
parent 388c3b6ad4
commit 77879deb5a
13 changed files with 2 additions and 1480 deletions

View File

@ -8,12 +8,12 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=librespeed-cli
PKG_VERSION:=1.0.13
PKG_VERSION:=1.0.14
PKG_RELEASE:=1
PKG_SOURCE:=$(PKG_NAME)-v$(PKG_VERSION).tar.gz
PKG_SOURCE_URL:=https://codeload.github.com/librespeed/speedtest-cli/tar.gz/v${PKG_VERSION}?
PKG_HASH:=5ad938b61e3edc0ca95e2ccff0c06e97a69383f3cbb0243bd47b21b9865f9f55
PKG_HASH:=3031e0f7babd7f9c51a1c49b95026d12532668455e8cc459049160f1bd525bf3
PKG_MAINTAINER:=Michal Vasilek <michal.vasilek@nic.cz>
PKG_LICENSE:=LGPL-3.0-only

View File

@ -1,61 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Fri, 7 Aug 2026 09:42:39 +0200
Subject: [PATCH] style: gofmt helper.go and speedtest.go
Import order, --simple block indentation and const alignment. No behaviour
change; split out to keep the next commit's diff readable.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
speedtest/helper.go | 16 ++++++++--------
speedtest/speedtest.go | 4 ++--
2 files changed, 10 insertions(+), 10 deletions(-)
--- a/speedtest/helper.go
+++ b/speedtest/helper.go
@@ -16,8 +16,8 @@ import (
"github.com/briandowns/spinner"
"github.com/gocarina/gocsv"
"github.com/librespeed/speedtest-cli/defs"
- "github.com/librespeed/speedtest-cli/report"
"github.com/librespeed/speedtest-cli/output"
+ "github.com/librespeed/speedtest-cli/report"
"github.com/urfave/cli/v2"
)
@@ -113,14 +113,14 @@ func doSpeedTest(c *cli.Context, servers
}
// print result if --simple is given
- if c.Bool(defs.OptionSimple) {
- if c.Bool(defs.OptionBytes) {
- useMebi := c.Bool(defs.OptionMebiBytes)
- output.WriteOut("Ping:\t%.2f ms\tJitter:\t%.2f ms\nDownload rate:\t%s\nUpload rate:\t%s\n", p, jitter, humanizeMbps(downloadValue, useMebi), humanizeMbps(uploadValue, useMebi))
- } else {
- output.WriteOut("Ping:\t%.2f ms\tJitter:\t%.2f ms\nDownload rate:\t%.2f Mbps\nUpload rate:\t%.2f Mbps\n", p, jitter, downloadValue, uploadValue)
+ if c.Bool(defs.OptionSimple) {
+ if c.Bool(defs.OptionBytes) {
+ useMebi := c.Bool(defs.OptionMebiBytes)
+ output.WriteOut("Ping:\t%.2f ms\tJitter:\t%.2f ms\nDownload rate:\t%s\nUpload rate:\t%s\n", p, jitter, humanizeMbps(downloadValue, useMebi), humanizeMbps(uploadValue, useMebi))
+ } else {
+ output.WriteOut("Ping:\t%.2f ms\tJitter:\t%.2f ms\nDownload rate:\t%.2f Mbps\nUpload rate:\t%.2f Mbps\n", p, jitter, downloadValue, uploadValue)
+ }
}
- }
// print share link if --share is given
var shareLink string
--- a/speedtest/speedtest.go
+++ b/speedtest/speedtest.go
@@ -33,8 +33,8 @@ const (
defaultTelemetryShare = "/results/"
forceNothing = 0
- forceHttps = 1
- forceHttp = 2
+ forceHttps = 1
+ forceHttp = 2
)
type PingJob struct {

View File

@ -1,526 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Fri, 7 Aug 2026 09:42:39 +0200
Subject: [PATCH] fix: fix data race, goroutine leak, non-TTY output and escape
injection
- BytesCounter.total mixed atomic and plain access; now an atomic.Uint64.
- Download/Upload leaked in-flight goroutines past "break Loop"; the done
signal is now ctx-aware and both wait on a WaitGroup before reading the
counter.
- Results rode on spinner.FinalMSG, which never prints without a TTY, so
piping produced no output at all. Printed via output.WriteUI now, and the
spinner moved off stdout onto stderr.
- Server-supplied strings were printed raw, allowing ANSI escapes and a
forged --list entry. output.Sanitize strips C0/DEL/C1 at every display
site; url.Parse only rejects ASCII controls, so C1 still needs stripping.
- BytesCounter.Read and its reader/pos/lock fields became unreachable when
Upload started building a body reader per request; dropped.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/bytes_counter.go | 47 ++++-------------
defs/server.go | 112 +++++++++++++++++++++++++++++------------
output/output.go | 24 +++++++++
speedtest/helper.go | 15 +++---
speedtest/speedtest.go | 12 +++--
5 files changed, 129 insertions(+), 81 deletions(-)
--- a/defs/bytes_counter.go
+++ b/defs/bytes_counter.go
@@ -1,55 +1,34 @@
package defs
import (
- "bytes"
"fmt"
"io"
"math/rand/v2"
- "sync"
"sync/atomic"
"time"
)
-// BytesCounter implements io.Reader and io.Writer interface, for counting bytes being read/written in HTTP requests
+// BytesCounter implements the io.Writer interface, for counting bytes being read/written in HTTP requests.
+// It is only ever plugged into an io.TeeReader, so the transfer itself is driven by the wrapped reader.
type BytesCounter struct {
start time.Time
- pos int
- total uint64
+ total atomic.Uint64
payload []byte
- reader io.ReadSeeker
mebi bool
uploadSize int
-
- lock *sync.Mutex
}
func NewCounter() *BytesCounter {
- return &BytesCounter{
- lock: &sync.Mutex{},
- }
+ return &BytesCounter{}
}
// Write implements io.Writer
func (c *BytesCounter) Write(p []byte) (int, error) {
n := len(p)
- atomic.AddUint64(&c.total, uint64(n))
+ c.total.Add(uint64(n))
return n, nil
}
-// Read implements io.Reader
-func (c *BytesCounter) Read(p []byte) (int, error) {
- c.lock.Lock()
- n, err := c.reader.Read(p)
- c.total += uint64(n)
- c.pos += n
- if c.pos == c.uploadSize {
- c.resetReader()
- }
- c.lock.Unlock()
-
- return n, err
-}
-
// SetBase sets the base for dividing bytes into megabyte or mebibyte
func (c *BytesCounter) SetMebi(mebi bool) {
c.mebi = mebi
@@ -62,7 +41,7 @@ func (c *BytesCounter) SetUploadSize(upl
// AvgBytes returns the average bytes/second
func (c *BytesCounter) AvgBytes() float64 {
- return float64(c.total) / time.Since(c.start).Seconds()
+ return float64(c.total.Load()) / time.Since(c.start).Seconds()
}
// AvgMbps returns the average mbits/second
@@ -99,17 +78,9 @@ func (c *BytesCounter) Payload() []byte
return c.payload
}
-// GenerateBlob generates a random byte array of `uploadSize` in the `payload` field, and sets the `reader` field to
-// read from it
+// GenerateBlob generates a random byte array of `uploadSize` in the `payload` field
func (c *BytesCounter) GenerateBlob() {
c.payload = getRandomData(c.uploadSize)
- c.reader = bytes.NewReader(c.payload)
-}
-
-// resetReader resets the `reader` field to 0 position
-func (c *BytesCounter) resetReader() (int64, error) {
- c.pos = 0
- return c.reader.Seek(0, 0)
}
// Start will set the `start` field to current time
@@ -119,12 +90,12 @@ func (c *BytesCounter) Start() {
// Total returns the total bytes read/written
func (c *BytesCounter) Total() uint64 {
- return atomic.LoadUint64(&c.total)
+ return c.total.Load()
}
// CurrentSpeed returns the current bytes/second
func (c *BytesCounter) CurrentSpeed() float64 {
- return float64(c.total) / time.Since(c.start).Seconds()
+ return float64(c.total.Load()) / time.Since(c.start).Seconds()
}
// SeekWrapper is a wrapper around io.Reader to give it a noop io.Seeker interface
--- a/defs/server.go
+++ b/defs/server.go
@@ -11,8 +11,10 @@ import (
"math"
"net/http"
"net/url"
+ "os"
"path"
"strconv"
+ "sync"
"time"
"github.com/briandowns/spinner"
@@ -61,7 +63,7 @@ func (s *Server) IsUp() bool {
defer resp.Body.Close()
b, err := io.ReadAll(resp.Body)
if err != nil || len(b) > 0 {
- output.WriteDebug("Failed when parsing get IP result: %s\n", b)
+ output.WriteDebug("Failed when parsing get IP result: %s\n", output.Sanitize(string(b)))
return false
}
// only return online if the ping URL returns nothing and 200
@@ -76,7 +78,7 @@ func (s *Server) ICMPPingAndJitter(count
}()
if s.NoICMP {
- output.WriteDebug("Skipping ICMP for server %s, will use HTTP ping\n", s.Name)
+ output.WriteDebug("Skipping ICMP for server %s, will use HTTP ping\n", output.Sanitize(s.Name))
return s.PingAndJitter(count + 2)
}
@@ -126,7 +128,7 @@ func (s *Server) ICMPPingAndJitter(count
if len(stats.Rtts) == 0 {
s.NoICMP = true
- output.WriteDebug("No ICMP pings returned for server %s (%s), trying TCP ping\n", s.Name, u.Hostname())
+ output.WriteDebug("No ICMP pings returned for server %s (%s), trying TCP ping\n", output.Sanitize(s.Name), output.Sanitize(u.Hostname()))
return s.PingAndJitter(count + 2)
}
@@ -226,29 +228,43 @@ func (s *Server) Download(silent bool, u
downloadDone := make(chan struct{}, requests)
+ var wg sync.WaitGroup
+
doDownload := func() {
+ defer wg.Done()
+
reqClone := req.Clone(ctx)
resp, err := http.DefaultClient.Do(reqClone)
if err != nil {
if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
output.WriteDebug("Failed when making HTTP request: %s\n", err)
}
- } else {
- defer resp.Body.Close()
+ return
+ }
+ defer resp.Body.Close()
- if _, err = io.Copy(io.Discard, io.TeeReader(resp.Body, counter)); err != nil {
- if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
- output.WriteDebug("Failed when reading HTTP response: %s\n", err)
- }
+ if _, err = io.Copy(io.Discard, io.TeeReader(resp.Body, counter)); err != nil {
+ if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
+ output.WriteDebug("Failed when reading HTTP response: %s\n", err)
}
+ }
- downloadDone <- struct{}{}
+ // let the main loop start a replacement request, but never block on it
+ // once the test is over, otherwise this goroutine is leaked
+ select {
+ case downloadDone <- struct{}{}:
+ case <-ctx.Done():
}
}
+ spawnDownload := func() {
+ wg.Add(1)
+ go doDownload()
+ }
+
counter.Start()
if !silent {
- pb := spinner.New(spinner.CharSets[11], 100*time.Millisecond)
+ pb := spinner.New(spinner.CharSets[11], 100*time.Millisecond, spinner.WithWriterFile(os.Stderr))
pb.Prefix = "Downloading... "
pb.PostUpdate = func(s *spinner.Spinner) {
if useBytes {
@@ -259,18 +275,21 @@ func (s *Server) Download(silent bool, u
}
pb.Start()
+ // print the rate ourselves instead of via pb.FinalMSG: the spinner only
+ // prints it when it was actually running, which it isn't when stderr is
+ // not a terminal
defer func() {
+ pb.Stop()
if useBytes {
- pb.FinalMSG = fmt.Sprintf("Download rate:\t%s\n", counter.AvgHumanize())
+ output.WriteUI("Download rate:\t%s\n", counter.AvgHumanize())
} else {
- pb.FinalMSG = fmt.Sprintf("Download rate:\t%.2f Mbps\n", counter.AvgMbps())
+ output.WriteUI("Download rate:\t%.2f Mbps\n", counter.AvgMbps())
}
- pb.Stop()
}()
}
for i := 0; i < requests; i++ {
- go doDownload()
+ spawnDownload()
time.Sleep(200 * time.Millisecond)
}
timeout := time.After(duration)
@@ -281,10 +300,14 @@ Loop:
cancel()
break Loop
case <-downloadDone:
- go doDownload()
+ spawnDownload()
}
}
+ // let the cancelled requests unwind before reading the counter, so the
+ // result doesn't change under us while it's being reported
+ wg.Wait()
+
return counter.AvgMbps(), counter.Total(), nil
}
@@ -301,8 +324,9 @@ func (s *Server) Upload(noPrealloc, sile
if noPrealloc {
output.WriteUI("Pre-allocation is disabled, performance might be lower!\n")
- counter.reader = &SeekWrapper{rand.Reader}
} else {
+ // each request reads from this shared payload; without it they stream
+ // straight from crypto/rand instead
counter.GenerateBlob()
}
@@ -318,7 +342,11 @@ func (s *Server) Upload(noPrealloc, sile
uploadDone := make(chan struct{}, requests)
+ var wg sync.WaitGroup
+
doUpload := func() {
+ defer wg.Done()
+
var bodyReader io.Reader
if noPrealloc {
bodyReader = &SeekWrapper{rand.Reader}
@@ -336,21 +364,34 @@ func (s *Server) Upload(noPrealloc, sile
uploadReq.Header.Set("Accept-Encoding", "identity")
resp, err := http.DefaultClient.Do(uploadReq)
- if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
- output.WriteDebug("Failed when making HTTP request: %s\n", err)
- } else if err == nil {
- defer resp.Body.Close()
- if _, err := io.Copy(io.Discard, resp.Body); err != nil {
- output.WriteDebug("Failed when reading HTTP response: %s\n", err)
+ if err != nil {
+ if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
+ output.WriteDebug("Failed when making HTTP request: %s\n", err)
}
+ return
+ }
+ defer resp.Body.Close()
+
+ if _, err := io.Copy(io.Discard, resp.Body); err != nil {
+ output.WriteDebug("Failed when reading HTTP response: %s\n", err)
+ }
- uploadDone <- struct{}{}
+ // let the main loop start a replacement request, but never block on it
+ // once the test is over, otherwise this goroutine is leaked
+ select {
+ case uploadDone <- struct{}{}:
+ case <-ctx.Done():
}
}
+ spawnUpload := func() {
+ wg.Add(1)
+ go doUpload()
+ }
+
counter.Start()
if !silent {
- pb := spinner.New(spinner.CharSets[11], 100*time.Millisecond)
+ pb := spinner.New(spinner.CharSets[11], 100*time.Millisecond, spinner.WithWriterFile(os.Stderr))
pb.Prefix = "Uploading... "
pb.PostUpdate = func(s *spinner.Spinner) {
if useBytes {
@@ -361,18 +402,21 @@ func (s *Server) Upload(noPrealloc, sile
}
pb.Start()
+ // print the rate ourselves instead of via pb.FinalMSG: the spinner only
+ // prints it when it was actually running, which it isn't when stderr is
+ // not a terminal
defer func() {
+ pb.Stop()
if useBytes {
- pb.FinalMSG = fmt.Sprintf("Upload rate:\t%s\n", counter.AvgHumanize())
+ output.WriteUI("Upload rate:\t%s\n", counter.AvgHumanize())
} else {
- pb.FinalMSG = fmt.Sprintf("Upload rate:\t%.2f Mbps\n", counter.AvgMbps())
+ output.WriteUI("Upload rate:\t%.2f Mbps\n", counter.AvgMbps())
}
- pb.Stop()
}()
}
for i := 0; i < requests; i++ {
- go doUpload()
+ spawnUpload()
time.Sleep(200 * time.Millisecond)
}
timeout := time.After(duration)
@@ -383,10 +427,14 @@ Loop:
cancel()
break Loop
case <-uploadDone:
- go doUpload()
+ spawnUpload()
}
}
+ // let the cancelled requests unwind before reading the counter, so the
+ // result doesn't change under us while it's being reported
+ wg.Wait()
+
return counter.AvgMbps(), counter.Total(), nil
}
@@ -432,7 +480,7 @@ func (s *Server) GetIPInfo(distanceUnit
if len(b) > 0 {
if err := json.Unmarshal(b, &ipInfo); err != nil {
output.WriteDebug("Failed when parsing get IP result: %s\n", err)
- output.WriteDebug("Received payload: %s\n", b)
+ output.WriteDebug("Received payload: %s\n", output.Sanitize(string(b)))
// try to extract processedString even if full parse fails
// (e.g. when rawIspInfo is "" instead of an object)
var partial struct {
@@ -473,7 +521,7 @@ func (s *Server) Sponsor() string {
if s.SponsorURL != "" {
su, err := url.Parse(s.SponsorURL)
if err != nil {
- output.WriteDebug("Sponsor URL is invalid: %s\n", s.SponsorURL)
+ output.WriteDebug("Sponsor URL is invalid: %s\n", output.Sanitize(s.SponsorURL))
} else {
if su.Scheme == "" {
su.Scheme = "https"
--- a/output/output.go
+++ b/output/output.go
@@ -11,6 +11,7 @@ import (
"fmt"
"io"
"os"
+ "strings"
)
// Default is the package-level output writer used by all package functions.
@@ -127,3 +128,26 @@ func (w *Writer) Fatalf(format string, a
fmt.Fprintln(w.ui)
os.Exit(1)
}
+
+// --- Sanitize: make server-supplied strings safe to display ---
+
+// Sanitize strips control characters from a string so it can be printed
+// without letting a remote party drive the terminal.
+//
+// Server names, sponsor strings and the getIP response all come off the wire
+// (over plain HTTP for schemeless servers), so they are attacker-influenced.
+// Left raw, an embedded ESC sequence can rewrite earlier lines, hide text or
+// recolour the output, and an embedded newline can forge an extra entry in
+// --list output that a script would then parse as real.
+//
+// Dropped: C0 controls (including ESC, CR, LF and TAB), DEL, and C1 controls
+// (0x80-0x9F, where 0x9B doubles as CSI on some terminals). Printable Unicode
+// is left alone.
+func Sanitize(s string) string {
+ return strings.Map(func(r rune) rune {
+ if r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f) {
+ return -1
+ }
+ return r
+ }, s)
+}
--- a/speedtest/helper.go
+++ b/speedtest/helper.go
@@ -46,10 +46,10 @@ func doSpeedTest(c *cli.Context, servers
return err
}
- output.WriteUI("Selected server: %s [%s]\n", currentServer.Name, u.Hostname())
+ output.WriteUI("Selected server: %s [%s]\n", output.Sanitize(currentServer.Name), output.Sanitize(u.Hostname()))
if sponsorMsg := currentServer.Sponsor(); sponsorMsg != "" {
- output.WriteUI("Sponsored by: %s\n", sponsorMsg)
+ output.WriteUI("Sponsored by: %s\n", output.Sanitize(sponsorMsg))
}
if currentServer.IsUp() {
@@ -58,12 +58,12 @@ func doSpeedTest(c *cli.Context, servers
output.WriteError("Failed to get IP info: %s\n", err)
return err
}
- output.WriteUI("You're testing from: %s\n", ispInfo.ProcessedString)
+ output.WriteUI("You're testing from: %s\n", output.Sanitize(ispInfo.ProcessedString))
// get ping and jitter value
var pb *spinner.Spinner
if !silent {
- pb = spinner.New(spinner.CharSets[11], 100*time.Millisecond)
+ pb = spinner.New(spinner.CharSets[11], 100*time.Millisecond, spinner.WithWriterFile(os.Stderr))
pb.Prefix = "Pinging server... "
pb.Start()
}
@@ -78,8 +78,11 @@ func doSpeedTest(c *cli.Context, servers
}
if pb != nil {
- pb.FinalMSG = fmt.Sprintf("Ping: %.2f ms\tJitter: %.2f ms\n", p, jitter)
+ // print the result ourselves instead of via pb.FinalMSG: the
+ // spinner only prints it when it was actually running, which it
+ // isn't when stderr is not a terminal
pb.Stop()
+ output.WriteUI("Ping: %.2f ms\tJitter: %.2f ms\n", p, jitter)
}
// get download value
@@ -182,7 +185,7 @@ func doSpeedTest(c *cli.Context, servers
reps_json = append(reps_json, rep)
}
} else {
- output.WriteUI("Selected server %s (%s) is not responding at the moment, try again later\n", currentServer.Name, u.Hostname())
+ output.WriteUI("Selected server %s (%s) is not responding at the moment, try again later\n", output.Sanitize(currentServer.Name), output.Sanitize(u.Hostname()))
}
//add a new line after each test if testing multiple servers
--- a/speedtest/speedtest.go
+++ b/speedtest/speedtest.go
@@ -283,9 +283,11 @@ func SpeedTest(c *cli.Context) error {
for _, svr := range servers {
var sponsorMsg string
if svr.Sponsor() != "" {
- sponsorMsg = fmt.Sprintf(" [Sponsor: %s]", svr.Sponsor())
+ sponsorMsg = fmt.Sprintf(" [Sponsor: %s]", output.Sanitize(svr.Sponsor()))
}
- output.WriteOut("%d: %s (%s) %s\n", svr.ID, svr.Name, svr.Server, sponsorMsg)
+ // --list goes to stdout, so a newline smuggled into a server name
+ // would forge an entry for anything parsing it
+ output.WriteOut("%d: %s (%s) %s\n", svr.ID, output.Sanitize(svr.Name), output.Sanitize(svr.Server), sponsorMsg)
}
return nil
}
@@ -360,7 +362,7 @@ func pingWorker(jobs <-chan PingJob, res
// get the URL of the speed test server from the JSON
u, err := server.GetURL()
if err != nil {
- output.WriteDebug("Server URL is invalid for %s (%s), skipping\n", server.Name, server.Server)
+ output.WriteDebug("Server URL is invalid for %s (%s), skipping\n", output.Sanitize(server.Name), output.Sanitize(server.Server))
wg.Done()
continue
}
@@ -373,7 +375,7 @@ func pingWorker(jobs <-chan PingJob, res
// if server is up, get ping
ping, _, err := server.ICMPPingAndJitter(1, srcIp, network)
if err != nil {
- output.WriteDebug("Can't ping server %s (%s), skipping\n", server.Name, u.Hostname())
+ output.WriteDebug("Can't ping server %s (%s), skipping\n", output.Sanitize(server.Name), output.Sanitize(u.Hostname()))
wg.Done()
continue
}
@@ -381,7 +383,7 @@ func pingWorker(jobs <-chan PingJob, res
results <- PingResult{Index: job.Index, Ping: ping}
wg.Done()
} else {
- output.WriteDebug("Server %s (%s) doesn't seem to be up, skipping\n", server.Name, u.Hostname())
+ output.WriteDebug("Server %s (%s) doesn't seem to be up, skipping\n", output.Sanitize(server.Name), output.Sanitize(u.Hostname()))
wg.Done()
}
}

View File

@ -1,321 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Fri, 7 Aug 2026 09:50:07 +0200
Subject: [PATCH] fix: align upload cancellation handling and add tests
- Upload's response-body copy now filters context.Canceled/DeadlineExceeded
the way the download path already did, so ending the test no longer logs a
spurious debug failure.
- The two raw-payload debug dumps use %q instead of Sanitize: newlines are
legitimate in a response body, and quoting escapes them without losing them.
- TestSanitize covers C0, DEL, C1 and the printable boundaries, exhaustively
rather than by example.
- TestBytesCounterConcurrentAccess reproduces the spinner-vs-transfer access
pattern. Verified: it reports DATA RACE against the pre-fix counter and
passes against the fixed one.
- Download and Upload are exercised against a server that never ends a
request, so a hanging wg.Wait() fails the test rather than a release.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/bytes_counter_test.go | 74 +++++++++++++++++++++++
defs/server.go | 12 +++-
defs/server_test.go | 120 +++++++++++++++++++++++++++++++++++++
output/output_test.go | 54 +++++++++++++++++
4 files changed, 257 insertions(+), 3 deletions(-)
create mode 100644 defs/bytes_counter_test.go
create mode 100644 defs/server_test.go
create mode 100644 output/output_test.go
--- /dev/null
+++ b/defs/bytes_counter_test.go
@@ -0,0 +1,74 @@
+package defs
+
+import (
+ "sync"
+ "testing"
+)
+
+// TestBytesCounterConcurrentAccess exercises the pattern the counter is used
+// in: several transfer goroutines writing while the progress spinner polls the
+// averages. It is the regression test for the mixed atomic/non-atomic access
+// that used to make this racy, so it is only meaningful under -race.
+func TestBytesCounterConcurrentAccess(t *testing.T) {
+ const (
+ writers = 8
+ writes = 2000
+ chunkSize = 16
+ pollers = 4
+ wantTotal = uint64(writers * writes * chunkSize)
+ pollBudget = 1 << 20
+ )
+
+ c := NewCounter()
+ c.Start()
+
+ stop := make(chan struct{})
+
+ var polling sync.WaitGroup
+ for i := 0; i < pollers; i++ {
+ polling.Add(1)
+ go func() {
+ defer polling.Done()
+ for n := 0; n < pollBudget; n++ {
+ select {
+ case <-stop:
+ return
+ default:
+ }
+ _ = c.AvgBytes()
+ _ = c.AvgMbps()
+ _ = c.AvgHumanize()
+ _ = c.CurrentSpeed()
+ _ = c.Total()
+ }
+ }()
+ }
+
+ var writing sync.WaitGroup
+ for i := 0; i < writers; i++ {
+ writing.Add(1)
+ go func() {
+ defer writing.Done()
+ chunk := make([]byte, chunkSize)
+ for j := 0; j < writes; j++ {
+ n, err := c.Write(chunk)
+ if err != nil {
+ t.Errorf("Write returned error: %v", err)
+ return
+ }
+ if n != chunkSize {
+ t.Errorf("Write returned %d, want %d", n, chunkSize)
+ return
+ }
+ }
+ }()
+ }
+
+ writing.Wait()
+ close(stop)
+ polling.Wait()
+
+ if got := c.Total(); got != wantTotal {
+ t.Errorf("Total() = %d, want %d (lost updates indicate a broken counter)", got, wantTotal)
+ }
+}
--- a/defs/server.go
+++ b/defs/server.go
@@ -63,7 +63,9 @@ func (s *Server) IsUp() bool {
defer resp.Body.Close()
b, err := io.ReadAll(resp.Body)
if err != nil || len(b) > 0 {
- output.WriteDebug("Failed when parsing get IP result: %s\n", output.Sanitize(string(b)))
+ // %q rather than Sanitize: this is a raw response body where newlines are
+ // legitimate, and quoting escapes control chars without losing them
+ output.WriteDebug("Failed when parsing get IP result: %q\n", b)
return false
}
// only return online if the ping URL returns nothing and 200
@@ -373,7 +375,10 @@ func (s *Server) Upload(noPrealloc, sile
defer resp.Body.Close()
if _, err := io.Copy(io.Discard, resp.Body); err != nil {
- output.WriteDebug("Failed when reading HTTP response: %s\n", err)
+ // cancellation is how the test ends, so it is not a failure
+ if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
+ output.WriteDebug("Failed when reading HTTP response: %s\n", err)
+ }
}
// let the main loop start a replacement request, but never block on it
@@ -480,7 +485,8 @@ func (s *Server) GetIPInfo(distanceUnit
if len(b) > 0 {
if err := json.Unmarshal(b, &ipInfo); err != nil {
output.WriteDebug("Failed when parsing get IP result: %s\n", err)
- output.WriteDebug("Received payload: %s\n", output.Sanitize(string(b)))
+ // %q rather than Sanitize: see IsUp
+ output.WriteDebug("Received payload: %q\n", b)
// try to extract processedString even if full parse fails
// (e.g. when rawIspInfo is "" instead of an object)
var partial struct {
--- /dev/null
+++ b/defs/server_test.go
@@ -0,0 +1,120 @@
+package defs
+
+import (
+ "io"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+ "time"
+)
+
+// The transfer tests below cover the timing semantics introduced by wg.Wait():
+// Download and Upload now return only once every in-flight request has
+// unwound. Against a server that never ends a request on its own, the only
+// thing that can end them is the context cancellation, so these fail (by
+// timing out) if wg.Wait() can hang.
+
+const (
+ testRequests = 2
+ // long enough that the requests are still in flight when the test ends
+ testDuration = 500 * time.Millisecond
+ // The spawn loop sleeps 200ms per request before the duration timer even
+ // starts, so a healthy run is ~900ms; measured unwind after cancel() is
+ // 10-40ms, with or without -race. The rest is slack for a loaded runner --
+ // kept tight so a hanging wg.Wait() fails fast instead of stalling CI.
+ returnBudget = testRequests*200*time.Millisecond + testDuration + 2*time.Second
+)
+
+// hangingDownloadServer streams forever until the client goes away.
+func hangingDownloadServer(t *testing.T) *httptest.Server {
+ t.Helper()
+ return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ chunk := make([]byte, 32*1024)
+ for {
+ select {
+ case <-r.Context().Done():
+ return
+ default:
+ }
+ if _, err := w.Write(chunk); err != nil {
+ return
+ }
+ if f, ok := w.(http.Flusher); ok {
+ f.Flush()
+ }
+ }
+ }))
+}
+
+// hangingUploadServer drains the body, then holds the request open.
+func hangingUploadServer(t *testing.T) *httptest.Server {
+ t.Helper()
+ return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ io.Copy(io.Discard, r.Body)
+ <-r.Context().Done()
+ }))
+}
+
+func runWithinBudget(t *testing.T, name string, fn func() error) {
+ t.Helper()
+
+ done := make(chan error, 1)
+ start := time.Now()
+ go func() { done <- fn() }()
+
+ select {
+ case err := <-done:
+ if err != nil {
+ t.Fatalf("%s returned error: %v", name, err)
+ }
+ if elapsed := time.Since(start); elapsed > returnBudget {
+ t.Errorf("%s took %s, budget was %s", name, elapsed, returnBudget)
+ }
+ case <-time.After(returnBudget):
+ t.Fatalf("%s did not return within %s: wg.Wait() is hanging on cancelled requests", name, returnBudget)
+ }
+}
+
+func TestDownloadReturnsWhenServerNeverEndsTheResponse(t *testing.T) {
+ ts := hangingDownloadServer(t)
+ defer ts.Close()
+
+ s := &Server{Server: ts.URL, DownloadURL: "/"}
+
+ var total uint64
+ runWithinBudget(t, "Download", func() error {
+ _, n, err := s.Download(true, false, false, testRequests, 100, testDuration)
+ total = n
+ return err
+ })
+
+ if total == 0 {
+ t.Error("Download reported 0 bytes, expected the counter to have seen traffic")
+ }
+}
+
+func TestUploadReturnsWhenServerNeverResponds(t *testing.T) {
+ ts := hangingUploadServer(t)
+ defer ts.Close()
+
+ s := &Server{Server: ts.URL, UploadURL: "/"}
+
+ runWithinBudget(t, "Upload", func() error {
+ _, _, err := s.Upload(false, true, false, false, testRequests, 32, testDuration)
+ return err
+ })
+}
+
+// The no-prealloc path streams from crypto/rand, so the request body never
+// ends on its own either; only cancellation can stop it.
+func TestUploadNoPreallocReturnsWhenServerNeverResponds(t *testing.T) {
+ ts := hangingUploadServer(t)
+ defer ts.Close()
+
+ s := &Server{Server: ts.URL, UploadURL: "/"}
+
+ runWithinBudget(t, "Upload(noPrealloc)", func() error {
+ _, _, err := s.Upload(true, true, false, false, testRequests, 32, testDuration)
+ return err
+ })
+}
--- /dev/null
+++ b/output/output_test.go
@@ -0,0 +1,54 @@
+package output
+
+import "testing"
+
+func TestSanitize(t *testing.T) {
+ tests := []struct {
+ name string
+ in string
+ want string
+ }{
+ {"empty", "", ""},
+ {"plain text is untouched", "normal text", "normal text"},
+ {"newline is dropped", "hello\nworld", "helloworld"},
+ {"carriage return is dropped", "hello\rworld", "helloworld"},
+ {"tab is dropped", "hello\tworld", "helloworld"},
+ {"nul is dropped", "hello\x00world", "helloworld"},
+ {"ANSI colour sequence loses its ESC", "\x1b[31mred\x1b[0m", "[31mred[0m"},
+ {"OSC window title sequence loses ESC and BEL", "\x1b]0;pwned\x07", "]0;pwned"},
+ {"DEL is dropped", "foo\x7fbar", "foobar"},
+ {"C1 CSI is dropped", "foo\u009b31mbar", "foo31mbar"},
+ {"C1 lower bound U+0080 is dropped", "foo\u0080bar", "foobar"},
+ {"C1 upper bound U+009F is dropped", "foo\u009fbar", "foobar"},
+ {"U+00A0 just past C1 is kept", "foo\u00a0bar", "foo\u00a0bar"},
+ {"space just past C0 is kept", "foo bar", "foo bar"},
+ {"non-ASCII text is kept", "Český server \U0001f1e8\U0001f1ff", "Český server \U0001f1e8\U0001f1ff"},
+ {"forged --list entry is collapsed onto one line", "Server A\n999: Fake", "Server A999: Fake"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ if got := Sanitize(tt.in); got != tt.want {
+ t.Errorf("Sanitize(%q) = %q, want %q", tt.in, got, tt.want)
+ }
+ })
+ }
+}
+
+// TestSanitizeDropsEveryControlRune asserts the boundaries exhaustively rather
+// than by example, so a future rewrite cannot silently let one class through.
+func TestSanitizeDropsEveryControlRune(t *testing.T) {
+ for r := rune(0); r <= 0x9f; r++ {
+ if r >= 0x20 && r <= 0x7e {
+ continue // printable ASCII
+ }
+ if got := Sanitize(string(r)); got != "" {
+ t.Errorf("Sanitize(%U) = %q, want it to be dropped", r, got)
+ }
+ }
+ for _, r := range []rune{0x20, 0x7e, 0xa0, 0xa1, 0x2028, 0x1f600} {
+ if got := Sanitize(string(r)); got != string(r) {
+ t.Errorf("Sanitize(%U) = %q, want it kept", r, got)
+ }
+ }
+}

View File

@ -1,130 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Sat, 8 Aug 2026 00:54:35 +0200
Subject: [PATCH] feat: report test progress under --debug
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
--json, --csv and --simple suppress the spinner, so a run prints nothing
between launch and the final result. --debug did not help: every existing
debug call sits on an error path, so a run that succeeds stays quiet.
Report each phase instead — server, IP info, ping, download, upload — with
the timings, byte counts and stream settings the spinner cannot carry.
Rates follow the run's own reporting mode, Mbps normally and MB/s under
--bytes, so a debug line cannot contradict the result printed beside it.
Nothing changes unless --debug is given.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
speedtest/helper.go | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
--- a/speedtest/helper.go
+++ b/speedtest/helper.go
@@ -47,18 +47,21 @@ func doSpeedTest(c *cli.Context, servers
}
output.WriteUI("Selected server: %s [%s]\n", output.Sanitize(currentServer.Name), output.Sanitize(u.Hostname()))
+ output.WriteDebug("Testing against %s (%s)\n", output.Sanitize(currentServer.Name), output.Sanitize(u.String()))
if sponsorMsg := currentServer.Sponsor(); sponsorMsg != "" {
output.WriteUI("Sponsored by: %s\n", output.Sanitize(sponsorMsg))
}
if currentServer.IsUp() {
+ output.WriteDebug("Fetching IP info\n")
ispInfo, err := currentServer.GetIPInfo(c.String(defs.OptionDistance))
if err != nil {
output.WriteError("Failed to get IP info: %s\n", err)
return err
}
output.WriteUI("You're testing from: %s\n", output.Sanitize(ispInfo.ProcessedString))
+ output.WriteDebug("IP info: %s\n", output.Sanitize(ispInfo.ProcessedString))
// get ping and jitter value
var pb *spinner.Spinner
@@ -71,12 +74,21 @@ func doSpeedTest(c *cli.Context, servers
// skip ICMP if option given
currentServer.NoICMP = noICMP
+ // The spinner is the only sign of progress, and it is not started
+ // in silent mode, so --json, --csv and --simple runs otherwise show
+ // nothing at all until they finish. Report each phase under --debug
+ // instead, with the timings and counts the spinner cannot carry.
+ output.WriteDebug("Ping test starting: %d pings, ICMP: %t\n", pingCount, !noICMP)
+ pingStart := time.Now()
+
p, jitter, err := currentServer.ICMPPingAndJitter(pingCount, c.String(defs.OptionSource), network)
if err != nil {
output.WriteError("Failed to get ping and jitter: %s\n", err)
return err
}
+ output.WriteDebug("Ping test finished in %s: ping %.2f ms, jitter %.2f ms\n", time.Since(pingStart).Round(time.Millisecond), p, jitter)
+
if pb != nil {
// print the result ourselves instead of via pb.FinalMSG: the
// spinner only prints it when it was actually running, which it
@@ -90,7 +102,11 @@ func doSpeedTest(c *cli.Context, servers
var bytesRead uint64
if c.Bool(defs.OptionNoDownload) {
output.WriteUI("Download test is disabled\n")
+ output.WriteDebug("Download test skipped\n")
} else {
+ output.WriteDebug("Download test starting: %d stream(s), %d chunk(s), up to %ds\n", c.Int(defs.OptionConcurrent), c.Int(defs.OptionChunks), c.Int(defs.OptionDuration))
+ downloadStart := time.Now()
+
download, br, err := currentServer.Download(silent, c.Bool(defs.OptionBytes), c.Bool(defs.OptionMebiBytes), c.Int(defs.OptionConcurrent), c.Int(defs.OptionChunks), time.Duration(c.Int(defs.OptionDuration))*time.Second)
if err != nil {
output.WriteError("Failed to get download speed: %s\n", err)
@@ -98,6 +114,8 @@ func doSpeedTest(c *cli.Context, servers
}
downloadValue = download
bytesRead = br
+
+ output.WriteDebug("Download test finished in %s: %s, %d byte(s) received\n", time.Since(downloadStart).Round(time.Millisecond), humanizeRate(download, c), br)
}
// get upload value
@@ -105,7 +123,11 @@ func doSpeedTest(c *cli.Context, servers
var bytesWritten uint64
if c.Bool(defs.OptionNoUpload) {
output.WriteUI("Upload test is disabled\n")
+ output.WriteDebug("Upload test skipped\n")
} else {
+ output.WriteDebug("Upload test starting: %d stream(s), %d KiB per request, up to %ds\n", c.Int(defs.OptionConcurrent), c.Int(defs.OptionUploadSize), c.Int(defs.OptionDuration))
+ uploadStart := time.Now()
+
upload, bw, err := currentServer.Upload(c.Bool(defs.OptionNoPreAllocate), silent, c.Bool(defs.OptionBytes), c.Bool(defs.OptionMebiBytes), c.Int(defs.OptionConcurrent), c.Int(defs.OptionUploadSize), time.Duration(c.Int(defs.OptionDuration))*time.Second)
if err != nil {
output.WriteError("Failed to get upload speed: %s\n", err)
@@ -113,6 +135,8 @@ func doSpeedTest(c *cli.Context, servers
}
uploadValue = upload
bytesWritten = bw
+
+ output.WriteDebug("Upload test finished in %s: %s, %d byte(s) sent\n", time.Since(uploadStart).Round(time.Millisecond), humanizeRate(upload, c), bw)
}
// print result if --simple is given
@@ -323,6 +347,16 @@ func sendTelemetry(telemetryServer defs.
}
}
+// humanizeRate formats a rate the same way the run's own result will be
+// reported, so a debug line cannot appear to contradict the JSON, CSV or
+// --simple output sitting next to it.
+func humanizeRate(mbps float64, c *cli.Context) string {
+ if c.Bool(defs.OptionBytes) {
+ return humanizeMbps(mbps, c.Bool(defs.OptionMebiBytes))
+ }
+ return fmt.Sprintf("%.2f Mbps", mbps)
+}
+
func humanizeMbps(mbps float64, useMebi bool) string {
val := mbps / 8
var base float64 = 1000

View File

@ -1,39 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Sat, 8 Aug 2026 00:57:36 +0200
Subject: [PATCH] docs: describe what --secure and --insecure actually do
Both said they applied "when communicating with LibreSpeed.org operated
servers". They apply to every server in the list, and they do not affect
fetching the list itself.
Help text only.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
main.go | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--- a/main.go
+++ b/main.go
@@ -150,13 +150,16 @@ func main() {
},
&cli.BoolFlag{
Name: defs.OptionSecure,
- Usage: "Use HTTPS instead of HTTP when communicating with\n" +
- "\tLibreSpeed.org operated servers",
+ Usage: "Force HTTPS for every test server, whichever scheme the\n" +
+ "\tserver list gives. Does not affect how the server list\n" +
+ "\titself is fetched",
},
&cli.BoolFlag{
Name: defs.OptionInsecure,
- Usage: "Use HTTP instead of HTTPS when communicating with\n" +
- "\tLibreSpeed.org operated servers",
+ Usage: "Force HTTP for every test server, whichever scheme the\n" +
+ "\tserver list gives. Does not affect how the server list\n" +
+ "\titself is fetched; use --" + defs.OptionServerJSON + " with an\n" +
+ "\thttp:// URL for that",
},
&cli.StringFlag{
Name: defs.OptionCACert,

View File

@ -1,33 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Sun, 9 Aug 2026 10:11:55 +0200
Subject: [PATCH] fix: terminate --json and --csv output with a newline
Neither ended with one: --json wrote the marshalled bytes as-is and --csv
explicitly trimmed the trailing newline. The shell prompt lands on the same
line as the result, and --csv-header already ends with a newline, so the two
CSV modes could not be concatenated into a valid file.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
speedtest/helper.go | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/speedtest/helper.go
+++ b/speedtest/helper.go
@@ -224,13 +224,14 @@ func doSpeedTest(c *cli.Context, servers
if err := gocsv.MarshalWithoutHeaders(&reps_csv, &buf); err != nil {
output.WriteError("Error generating CSV report: %s\n", err)
} else {
- os.Stdout.WriteString(strings.TrimRight(buf.String(), "\n\r"))
+ os.Stdout.WriteString(strings.TrimRight(buf.String(), "\n\r") + "\n")
}
} else if c.Bool(defs.OptionJSON) {
if b, err := json.Marshal(&reps_json); err != nil {
output.WriteError("Error generating JSON report: %s\n", err)
} else {
os.Stdout.Write(b[:])
+ os.Stdout.WriteString("\n")
}
}

View File

@ -1,23 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Sun, 9 Aug 2026 10:11:55 +0200
Subject: [PATCH] fix: drop the empty copyright line from --version
"librespeed.org Copyright (C)" has named no holder and no year since the
first commit. It says nothing that the two lines above it do not.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
speedtest/speedtest.go | 1 -
1 file changed, 1 deletion(-)
--- a/speedtest/speedtest.go
+++ b/speedtest/speedtest.go
@@ -73,7 +73,6 @@ func SpeedTest(c *cli.Context) error {
output.WriteOut("Licensed under GNU Lesser General Public License v3.0\n")
output.WriteOut("LibreSpeed\tCopyright (C) 2016-2020 Federico Dossena\n")
output.WriteOut("librespeed-cli\tCopyright (C) 2020 Maddie Zhan\n")
- output.WriteOut("librespeed.org\tCopyright (C)\n")
return nil
}

View File

@ -1,54 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Sun, 9 Aug 2026 23:27:01 +0200
Subject: [PATCH] feat: report the negotiated TLS version and cipher under
--debug
On hardware without AES acceleration the cipher, not the link, is what
bounds an HTTPS result, and the server chooses it. Two runs can differ
several-fold for a reason the numbers alone do not show.
Measured on a CZ.NIC Turris 1.x, whose e500v2 core has no crypto
instructions: AES-256-GCM tops out around 150 Mbps while ChaCha20-Poly1305
reaches about 390 Mbps on the same core. A server that picks the first will
look like a slow link next to one that picks the second.
Print it once, from the backend check that already runs before every test.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/server.go | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
--- a/defs/server.go
+++ b/defs/server.go
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"crypto/rand"
+ "crypto/tls"
"encoding/json"
"errors"
"fmt"
@@ -61,6 +62,21 @@ func (s *Server) IsUp() bool {
return false
}
defer resp.Body.Close()
+
+ // Report what the connection actually negotiated. On hardware without AES
+ // acceleration the cipher, not the link, is what bounds the result, and
+ // under TLS 1.3 the server picks it: the client offers a set and has no say
+ // in the choice, and Go does not allow that set to be configured at all.
+ // Two runs can therefore differ several-fold for a reason the numbers alone
+ // do not show, which is what this line is for.
+ if resp.TLS != nil {
+ output.WriteDebug("Negotiated %s with %s\n",
+ tls.VersionName(resp.TLS.Version),
+ tls.CipherSuiteName(resp.TLS.CipherSuite))
+ } else {
+ output.WriteDebug("Connection is not encrypted\n")
+ }
+
b, err := io.ReadAll(resp.Body)
if err != nil || len(b) > 0 {
// %q rather than Sanitize: this is a raw response body where newlines are

View File

@ -1,120 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Mon, 10 Aug 2026 01:03:30 +0200
Subject: [PATCH] feat: report which address each ping test used under --debug
IPv4 and IPv6 can take different paths through the network, so a result is
not fully described by the hostname it was measured against. Nothing in the
output says which was used: --json carries no client address, and the
human-readable line only shows it by accident, in the ISP string.
It is not academic. Against speedtest.cesnet.cz the ICMP ping goes over
IPv4 while the transfers go over IPv6, so the reported latency and the
reported throughput describe two different paths.
The ICMP path reports the address the pinger resolved and probed; the TCP
path reports every distinct peer the sockets connected to, taken from
httptrace. The requests usually share one connection, but a reconnect can
land on a different address, and reporting only the first would describe a
connection the later samples did not use.
Both paths are covered because --no-icmp is common enough that doing only
the ICMP one would leave most users with nothing.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/server.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
--- a/defs/server.go
+++ b/defs/server.go
@@ -10,10 +10,13 @@ import (
"fmt"
"io"
"math"
+ "net"
"net/http"
+ "net/http/httptrace"
"net/url"
"os"
"path"
+ "slices"
"strconv"
"sync"
"time"
@@ -129,6 +132,15 @@ func (s *Server) ICMPPingAndJitter(count
stats := p.Statistics()
+ // Say which address the test actually reached. IPv4 and IPv6 can take
+ // different paths through the network, so a result is not fully described
+ // by the hostname it was measured against, and --json carries no client
+ // address to infer it from.
+ if stats.IPAddr != nil {
+ output.WriteDebug("Pinging %s over ICMP (%s)\n",
+ stats.IPAddr.String(), addressFamily(stats.IPAddr.String()))
+ }
+
var lastPing, jitter float64
for idx, rtt := range stats.Rtts {
if idx != 0 {
@@ -153,6 +165,24 @@ func (s *Server) ICMPPingAndJitter(count
return float64(stats.AvgRtt.Milliseconds()), jitter, nil
}
+// addressFamily names the IP version of an address, for reporting which path a
+// measurement took. Accepts either a bare address or one with a port.
+func addressFamily(addr string) string {
+ host := addr
+ if h, _, err := net.SplitHostPort(addr); err == nil {
+ host = h
+ }
+ ip := net.ParseIP(host)
+ switch {
+ case ip == nil:
+ return "unknown"
+ case ip.To4() != nil:
+ return "IPv4"
+ default:
+ return "IPv6"
+ }
+}
+
// PingAndJitter pings the server via accessing ping URL and calculate the average ping and jitter
func (s *Server) PingAndJitter(count int) (float64, float64, error) {
t := time.Now()
@@ -176,6 +206,23 @@ func (s *Server) PingAndJitter(count int
}
req.Header.Set("User-Agent", UserAgent)
+ // Collect every distinct peer, not just the first. The requests usually
+ // share one connection, but a reconnect can land on a different address --
+ // a different family, even -- and reporting only the first would then
+ // describe a connection the later samples did not use.
+ var remotes []string
+ req = req.WithContext(httptrace.WithClientTrace(req.Context(), &httptrace.ClientTrace{
+ GotConn: func(info httptrace.GotConnInfo) {
+ if info.Conn == nil {
+ return
+ }
+ addr := info.Conn.RemoteAddr().String()
+ if !slices.Contains(remotes, addr) {
+ remotes = append(remotes, addr)
+ }
+ },
+ }))
+
for i := 0; i < count; i++ {
start := time.Now()
resp, err := http.DefaultClient.Do(req)
@@ -190,6 +237,10 @@ func (s *Server) PingAndJitter(count int
pings = append(pings, float64(end.Sub(start).Milliseconds()))
}
+ for _, addr := range remotes {
+ output.WriteDebug("Pinging %s over TCP (%s)\n", addr, addressFamily(addr))
+ }
+
// discard first result due to handshake overhead
if len(pings) > 1 {
pings = pings[1:]

View File

@ -1,81 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe@bloodkings.eu>
Date: Mon, 10 Aug 2026 01:35:20 +0200
Subject: [PATCH] fix: keep sub-millisecond precision in ping and jitter
Duration.Milliseconds truncates to whole milliseconds, and every round-trip
sample went through it -- in the ICMP path and in the TCP fallback that
--no-icmp selects. On a link faster than a millisecond that leaves nothing.
Against a server on loopback, reporting only ping and jitter:
before ping 0 jitter 0
after ping 0.11 jitter 0.01
Over a wider link the reported ping still looks fractional, because it is an
average of the samples, but its resolution is a whole millisecond and jitter
is a difference of those samples. Three consecutive ICMP runs against the
same server:
before ping 5, 6, 5 jitter 0.59, 4.40, 0.02
after ping 5.48, 5.68, 5.45 jitter 0.27, 0.32, 0.12
Divide by time.Millisecond instead, which keeps the fraction.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/server.go | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
--- a/defs/server.go
+++ b/defs/server.go
@@ -91,6 +91,13 @@ func (s *Server) IsUp() bool {
return resp.StatusCode == http.StatusOK
}
+// rttMillis converts a round-trip time to milliseconds without discarding the
+// fraction. Duration.Milliseconds truncates to whole milliseconds, which on a
+// link faster than that leaves nothing to report.
+func rttMillis(d time.Duration) float64 {
+ return float64(d) / float64(time.Millisecond)
+}
+
// ICMPPingAndJitter pings the server via ICMP echos and calculate the average ping and jitter
func (s *Server) ICMPPingAndJitter(count int, srcIp, network string) (float64, float64, error) {
t := time.Now()
@@ -144,7 +151,7 @@ func (s *Server) ICMPPingAndJitter(count
var lastPing, jitter float64
for idx, rtt := range stats.Rtts {
if idx != 0 {
- instJitter := math.Abs(lastPing - float64(rtt.Milliseconds()))
+ instJitter := math.Abs(lastPing - rttMillis(rtt))
if idx > 1 {
if jitter > instJitter {
jitter = jitter*0.7 + instJitter*0.3
@@ -153,7 +160,7 @@ func (s *Server) ICMPPingAndJitter(count
}
}
}
- lastPing = float64(rtt.Milliseconds())
+ lastPing = rttMillis(rtt)
}
if len(stats.Rtts) == 0 {
@@ -162,7 +169,7 @@ func (s *Server) ICMPPingAndJitter(count
return s.PingAndJitter(count + 2)
}
- return float64(stats.AvgRtt.Milliseconds()), jitter, nil
+ return rttMillis(stats.AvgRtt), jitter, nil
}
// addressFamily names the IP version of an address, for reporting which path a
@@ -234,7 +241,7 @@ func (s *Server) PingAndJitter(count int
resp.Body.Close()
end := time.Now()
- pings = append(pings, float64(end.Sub(start).Milliseconds()))
+ pings = append(pings, rttMillis(end.Sub(start)))
}
for _, addr := range remotes {

View File

@ -1,45 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Mon, 10 Aug 2026 02:11:41 +0200
Subject: [PATCH] feat: report the ping distribution under --debug
The test reports one average, which says nothing about how the samples were
spread. A link averaging 5 ms because every probe took 5 ms and one averaging
5 ms because probes ranged from 1 to 30 ms are not the same link, and only the
second explains a connection that feels unsteady.
pro-bing already works min, max and standard deviation out in Statistics(),
so this prints what is on hand.
Replies are shown as a raw count rather than a loss percentage. Ten probes are
too few to express as a rate, and ICMP is frequently policed separately from
the data path, so a percentage would describe the server's ICMP handling more
than the network.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/server.go | 13 +++++++++++++
1 file changed, 13 insertions(+)
--- a/defs/server.go
+++ b/defs/server.go
@@ -148,6 +148,19 @@ func (s *Server) ICMPPingAndJitter(count
stats.IPAddr.String(), addressFamily(stats.IPAddr.String()))
}
+ // A single figure hides how the samples were spread. The pinger already
+ // works these out, and the spread is what says whether a link is steady or
+ // merely fast on average. Raw counts rather than a loss percentage: ten
+ // probes are too few for a rate, and ICMP is often policed independently of
+ // the data path, so a percentage would say more about the server's ICMP
+ // handling than about the network.
+ output.WriteDebug("Ping over ICMP: min %.2f ms, avg %.2f ms, max %.2f ms, stddev %.2f ms, %d/%d replies\n",
+ rttMillis(stats.MinRtt),
+ rttMillis(stats.AvgRtt),
+ rttMillis(stats.MaxRtt),
+ rttMillis(stats.StdDevRtt),
+ stats.PacketsRecv, stats.PacketsSent)
+
var lastPing, jitter float64
for idx, rtt := range stats.Rtts {
if idx != 0 {

View File

@ -1,45 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Mon, 10 Aug 2026 08:36:47 +0200
Subject: [PATCH] feat: name the platform in the User-Agent
A server's telemetry stores the User-Agent header already, but the client only
identifies itself by name and version, so an operator cannot tell which kinds
of machine are measuring against them. Browsers have always reported this.
Append the operating system and architecture, which runtime.GOOS and
runtime.GOARCH give as compile-time constants at no cost:
librespeed-cli/1.0.12 (linux; arm64)
It stays coarse on purpose. The kernel version or the hostname would identify
the machine rather than describe it, and nothing here needs that.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
Upstream-Status: Submitted
---
defs/defs.go | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
--- a/defs/defs.go
+++ b/defs/defs.go
@@ -1,11 +1,19 @@
package defs
+import "runtime"
+
var (
// values to be filled in by build script
BuildDate string
ProgName string
ProgVersion string
- UserAgent = ProgName + "/" + ProgVersion
+ // UserAgent names the platform as well as the program, the way a browser
+ // does. A server's telemetry already stores this header, so reporting the
+ // operating system and architecture here is what lets an operator tell
+ // which kinds of machine are measuring against them without any change to
+ // what the client sends. It stays coarse deliberately: the kernel version
+ // or the hostname would identify the machine rather than describe it.
+ UserAgent = ProgName + "/" + ProgVersion + " (" + runtime.GOOS + "; " + runtime.GOARCH + ")"
)
// GetIPResults represents the returned JSON from backend server's getIP.php endpoint