From: Josef Schlehofer Date: Mon, 10 Aug 2026 08:46:16 +0000 (+0200) Subject: librespeed-cli: update to 1.0.13 X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=f61a21e47213cda66781d49e77c832a16b49ad5d;p=openwrt-packages.git librespeed-cli: update to 1.0.13 Carry fixes for a data race, a goroutine leak, output escaping, JSON and CSV termination, and sub-millisecond ping precision, plus additional --debug output. All patches are under review upstream. Set the version through GO_PKG_LDFLAGS_X. The package never passed one, so --version reported neither a name nor a version. Drop the serverIdx patch: 1.0.13 resolves the same problem by checking the map lookup rather than reading a possibly absent key. Signed-off-by: Josef Schlehofer --- diff --git a/utils/librespeed-cli/Makefile b/utils/librespeed-cli/Makefile index 0bca5e637..ff1be9c66 100644 --- a/utils/librespeed-cli/Makefile +++ b/utils/librespeed-cli/Makefile @@ -8,12 +8,12 @@ include $(TOPDIR)/rules.mk PKG_NAME:=librespeed-cli -PKG_VERSION:=1.0.12 +PKG_VERSION:=1.0.13 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:=2813ff49a8bc99687e70599212e05f0d995cd7f685e1202e80eea4ff58767301 +PKG_HASH:=5ad938b61e3edc0ca95e2ccff0c06e97a69383f3cbb0243bd47b21b9865f9f55 PKG_MAINTAINER:=Michal Vasilek PKG_LICENSE:=LGPL-3.0-only @@ -25,6 +25,15 @@ PKG_BUILD_PARALLEL:=1 PKG_BUILD_FLAGS:=no-mips16 GO_PKG:=github.com/librespeed/speedtest-cli +# Without these the binary reports no name and no version at all, which leaves +# --version printing an empty string where the version belongs. The v prefix +# matches what the upstream release build produces, so the same version does +# not read differently depending on where the binary came from. BuildDate is +# left unset on purpose: the only value available here is a timestamp, and +# baking one in would make the build unreproducible. +GO_PKG_LDFLAGS_X:= \ + $(GO_PKG)/defs.ProgName=$(PKG_NAME) \ + $(GO_PKG)/defs.ProgVersion=v$(PKG_VERSION) include $(INCLUDE_DIR)/package.mk include ../../lang/golang/golang-package.mk diff --git a/utils/librespeed-cli/patches/0001-speedtest-fix-start-serverIdx-in-pingList-when-some-servers-are-down.patch b/utils/librespeed-cli/patches/0001-speedtest-fix-start-serverIdx-in-pingList-when-some-servers-are-down.patch deleted file mode 100644 index e51a007ae..000000000 --- a/utils/librespeed-cli/patches/0001-speedtest-fix-start-serverIdx-in-pingList-when-some-servers-are-down.patch +++ /dev/null @@ -1,22 +0,0 @@ -From b8fda15c50575ad7f1086fc8b0561b98e65c9c1e Mon Sep 17 00:00:00 2001 -From: user -Date: Wed, 18 Dec 2024 21:36:23 +0800 -Subject: [PATCH] fix start serverIdx in pingList when some servers are down - ---- - speedtest/speedtest.go | 4 ++++ - 1 file changed, 4 insertions(+) - ---- a/speedtest/speedtest.go -+++ b/speedtest/speedtest.go -@@ -321,6 +321,10 @@ func SpeedTest(c *cli.Context) error { - - // get the fastest server's index in the `servers` array - var serverIdx int -+ for serverIdx = range pingList { -+ break -+ } -+ - for idx, ping := range pingList { - if ping > 0 && ping <= pingList[serverIdx] { - serverIdx = idx diff --git a/utils/librespeed-cli/patches/0001-style-gofmt-helper.go-and-speedtest.go.patch b/utils/librespeed-cli/patches/0001-style-gofmt-helper.go-and-speedtest.go.patch new file mode 100644 index 000000000..3301a240e --- /dev/null +++ b/utils/librespeed-cli/patches/0001-style-gofmt-helper.go-and-speedtest.go.patch @@ -0,0 +1,61 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 { diff --git a/utils/librespeed-cli/patches/0002-fix-fix-data-race-goroutine-leak-non-TTY-output-and-.patch b/utils/librespeed-cli/patches/0002-fix-fix-data-race-goroutine-leak-non-TTY-output-and-.patch new file mode 100644 index 000000000..23ca77ae4 --- /dev/null +++ b/utils/librespeed-cli/patches/0002-fix-fix-data-race-goroutine-leak-non-TTY-output-and-.patch @@ -0,0 +1,526 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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() + } + } diff --git a/utils/librespeed-cli/patches/0003-fix-align-upload-cancellation-handling-and-add-tests.patch b/utils/librespeed-cli/patches/0003-fix-align-upload-cancellation-handling-and-add-tests.patch new file mode 100644 index 000000000..46a86ee6b --- /dev/null +++ b/utils/librespeed-cli/patches/0003-fix-align-upload-cancellation-handling-and-add-tests.patch @@ -0,0 +1,321 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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) ++ } ++ } ++} diff --git a/utils/librespeed-cli/patches/0004-feat-report-test-progress-under-debug.patch b/utils/librespeed-cli/patches/0004-feat-report-test-progress-under-debug.patch new file mode 100644 index 000000000..559a442ed --- /dev/null +++ b/utils/librespeed-cli/patches/0004-feat-report-test-progress-under-debug.patch @@ -0,0 +1,130 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 diff --git a/utils/librespeed-cli/patches/0005-docs-describe-what-secure-and-insecure-actually-do.patch b/utils/librespeed-cli/patches/0005-docs-describe-what-secure-and-insecure-actually-do.patch new file mode 100644 index 000000000..73617883a --- /dev/null +++ b/utils/librespeed-cli/patches/0005-docs-describe-what-secure-and-insecure-actually-do.patch @@ -0,0 +1,39 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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, diff --git a/utils/librespeed-cli/patches/0006-fix-terminate-json-and-csv-output-with-a-newline.patch b/utils/librespeed-cli/patches/0006-fix-terminate-json-and-csv-output-with-a-newline.patch new file mode 100644 index 000000000..750466c83 --- /dev/null +++ b/utils/librespeed-cli/patches/0006-fix-terminate-json-and-csv-output-with-a-newline.patch @@ -0,0 +1,33 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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") + } + } + diff --git a/utils/librespeed-cli/patches/0007-fix-drop-the-empty-copyright-line-from-version.patch b/utils/librespeed-cli/patches/0007-fix-drop-the-empty-copyright-line-from-version.patch new file mode 100644 index 000000000..9347c28cd --- /dev/null +++ b/utils/librespeed-cli/patches/0007-fix-drop-the-empty-copyright-line-from-version.patch @@ -0,0 +1,23 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 + } + diff --git a/utils/librespeed-cli/patches/0008-feat-report-the-negotiated-TLS-version-and-cipher-un.patch b/utils/librespeed-cli/patches/0008-feat-report-the-negotiated-TLS-version-and-cipher-un.patch new file mode 100644 index 000000000..b950b595f --- /dev/null +++ b/utils/librespeed-cli/patches/0008-feat-report-the-negotiated-TLS-version-and-cipher-un.patch @@ -0,0 +1,54 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 diff --git a/utils/librespeed-cli/patches/0009-feat-report-which-address-each-ping-test-used-under-.patch b/utils/librespeed-cli/patches/0009-feat-report-which-address-each-ping-test-used-under-.patch new file mode 100644 index 000000000..20d7d28a0 --- /dev/null +++ b/utils/librespeed-cli/patches/0009-feat-report-which-address-each-ping-test-used-under-.patch @@ -0,0 +1,120 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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:] diff --git a/utils/librespeed-cli/patches/0010-fix-keep-sub-millisecond-precision-in-ping-and-jitte.patch b/utils/librespeed-cli/patches/0010-fix-keep-sub-millisecond-precision-in-ping-and-jitte.patch new file mode 100644 index 000000000..68e7b4b50 --- /dev/null +++ b/utils/librespeed-cli/patches/0010-fix-keep-sub-millisecond-precision-in-ping-and-jitte.patch @@ -0,0 +1,81 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 { diff --git a/utils/librespeed-cli/patches/0011-feat-report-the-ping-distribution-under-debug.patch b/utils/librespeed-cli/patches/0011-feat-report-the-ping-distribution-under-debug.patch new file mode 100644 index 000000000..119b5c799 --- /dev/null +++ b/utils/librespeed-cli/patches/0011-feat-report-the-ping-distribution-under-debug.patch @@ -0,0 +1,45 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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 { diff --git a/utils/librespeed-cli/patches/0012-feat-name-the-platform-in-the-User-Agent.patch b/utils/librespeed-cli/patches/0012-feat-name-the-platform-in-the-User-Agent.patch new file mode 100644 index 000000000..ba3ec999f --- /dev/null +++ b/utils/librespeed-cli/patches/0012-feat-name-the-platform-in-the-User-Agent.patch @@ -0,0 +1,45 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Josef Schlehofer +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 +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