From cb248cc1dc8b516f10d34fbb529c3b48446129e6 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Tue, 11 Aug 2026 23:47:50 +0200 Subject: [PATCH] Download: do not modify the caller's options record Filling in the defaults for 'verifyCert' and 'maxTime' wrote into the record the caller passed in, so a record reused across several calls kept the settings from the first one. Only visible when the preferences differ from their defaults -- with the shipped values neither branch is taken -- which is why the test sets DownloadMaxTime first. Co-Authored-By: Claude Opus 5 --- lib/download.gi | 3 +++ tst/download.tst | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/download.gi b/lib/download.gi index 1e01b2e..f3bf36c 100644 --- a/lib/download.gi +++ b/lib/download.gi @@ -247,6 +247,9 @@ InstallMethod( Download, function( url, opt ) local timeout, errors, r, res; + # Do not modify the caller's record when filling in the defaults below. + opt:= ShallowCopy( opt ); + # Set the default for 'verifyCert' if necessary. if not IsBound( opt.verifyCert ) and UserPreference( "utils", "DownloadVerifyCertificate" ) = false then diff --git a/tst/download.tst b/tst/download.tst index e52cc6e..5f52849 100644 --- a/tst/download.tst +++ b/tst/download.tst @@ -1,4 +1,4 @@ -#@local meths, i, urls, pair, url, expected, res1, good1, n, file, res2, good2, contents, r, res3, good3, bad, server, baseurl, iometh +#@local meths, i, urls, pair, url, expected, res1, good1, n, file, res2, good2, contents, r, res3, good3, bad, server, baseurl, iometh, opt, oldpref ############################################################################ ## #W download.tst Utils Package Thomas Breuer @@ -139,6 +139,19 @@ gap> res1:= Download( url, rec( maxTime:= 5 ) );; gap> res1.success = true; true +## 'Download' must not modify the given options record. +## The defaults are filled in only when the preferences differ from their +## default values, hence the 'SetUserPreference' call. +gap> oldpref:= UserPreference( "utils", "DownloadMaxTime" );; +gap> SetUserPreference( "utils", "DownloadMaxTime", 30 ); +gap> opt:= rec();; +gap> res1:= Download( Concatenation( baseurl, "/success" ), opt );; +gap> res1.success; +true +gap> RecNames( opt ); +[ ] +gap> SetUserPreference( "utils", "DownloadMaxTime", oldpref ); + ## test errors and redirects gap> res1:= Download( Concatenation( baseurl, "/missing" ) );; gap> res1.success = false;