From e1eec7df27f5b28305a000372a2076870b9ef9ff Mon Sep 17 00:00:00 2001 From: Max Horn Date: Wed, 12 Aug 2026 08:42:49 +0200 Subject: [PATCH] Add a 'target' option for writing the response to a file 'DownloadURL' and friends built the whole response body as a GAP string and returned it, so the largest file one could fetch was bounded by memory. That matters for the use case this package is increasingly put to: fetching data sets on behalf of other packages. With opt.target set to a filename, the body is written straight to that file via CURLOPT_WRITEDATA as it arrives, and the result record has no 'result' component. Downloading 200 MB now costs 158 MB peak RSS instead of 334 MB. If the request fails the file is removed, so a caller may test whether it exists to decide whether it got the data. Note that CURL_REQUEST now takes 9 arguments rather than 8. It is not documented and CurlRequest fills in the new one, but anyone calling the kernel function directly has to adjust. Co-Authored-By: Claude Opus 5 --- gap/curl.gd | 7 +++++- gap/curl.gi | 8 +++++-- src/curl.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++---- tst/basic.tst | 44 +++++++++++++++++++++++++++++++++- tst/errors.tst | 2 +- 5 files changed, 116 insertions(+), 10 deletions(-) diff --git a/gap/curl.gd b/gap/curl.gd index 1b71149..e200ec1 100644 --- a/gap/curl.gd +++ b/gap/curl.gd @@ -136,13 +136,18 @@ DeclareGlobalFunction("DeleteURL"); #! the default is false). #! * maxTime: Maximum time in seconds that you allow each transfer #! to take. 0 means no limitation. (default 0). +#! * target: the name of a file to write the body of the response +#! to, as a string, or false to have it returned as a string +#! (the default). The data is written as it arrives, so the size of +#! the response is not limited by the available memory. If the request +#! fails, the file is not left behind. #! #! As output, this function returns a record containing some of the following #! components, which describe the outcome of the request: #! * success: a boolean describing whether the request was #! successfully received by the server; #! * result: body of the information sent by the server (only if -#! success = true); +#! success = true and no target was given); #! * error: human-readable string saying what went wrong (only if #! success = false). #! diff --git a/gap/curl.gi b/gap/curl.gi index 3064166..eb2b114 100644 --- a/gap/curl.gi +++ b/gap/curl.gi @@ -9,7 +9,7 @@ function(URL, type, out_string, opts...) # Get options r := rec(verifyCert := true, verbose := false, followRedirect := true, - failOnError:= false, maxTime := 0); + failOnError:= false, maxTime := 0, target := false); if Length(opts) = 1 then if not IsRecord(opts[1]) then ErrorNoReturn("CurlRequest: must be a record"); @@ -42,13 +42,17 @@ function(URL, type, out_string, opts...) " must be a non-negative integer"); fi; od; + if r.target <> false and not IsString(r.target) then + ErrorNoReturn("CurlRequest: .target must be a string or false"); + fi; return CURL_REQUEST(URL, type, out_string, r.verifyCert, r.verbose, r.followRedirect, r.failOnError, - r.maxTime); + r.maxTime, + r.target); end); InstallGlobalFunction("DownloadURL", diff --git a/src/curl.c b/src/curl.c index c5b6a1e..0d575a9 100644 --- a/src/curl.c +++ b/src/curl.c @@ -33,6 +33,12 @@ size_t write_string(char * ptr, size_t size, size_t nmemb, void * outstream) return size * nmemb; } +// Write straight to a file, for CURLOPT_WRITEDATA when a target is given. +size_t write_file(char * ptr, size_t size, size_t nmemb, void * outstream) +{ + return fwrite(ptr, size, nmemb, (FILE *)outstream); +} + Obj FuncCURL_REQUEST(Obj self, Obj input_list) { CURL * curl; @@ -42,9 +48,11 @@ Obj FuncCURL_REQUEST(Obj self, Obj input_list) curl_off_t len; char urlbuf[4096] = { 0 }; char * typebuf = NULL; + char * targetbuf = NULL; + FILE * targetfile = NULL; const int n = LEN_PLIST(input_list); - GAP_ASSERT(n == 8); // paranoia check, GAP enforces this + GAP_ASSERT(n == 9); // paranoia check, GAP enforces this Obj URL = ELM_PLIST(input_list, 1); if (!IS_STRING_REP(URL)) { @@ -70,6 +78,31 @@ Obj FuncCURL_REQUEST(Obj self, Obj input_list) } memcpy(urlbuf, CONST_CSTR_STRING(URL), len); + // If a target file was given, write the body straight into it instead of + // building it up in memory. Copy the name out of the GAP string for the + // same reason as the URL above. + Obj target = ELM_PLIST(input_list, 9); + if (target != False) { + if (!IS_STRING_REP(target)) { + target = CopyToStringRep(target); + } + len = GET_LEN_STRING(target) + 1; + targetbuf = (char *)malloc(len); + memcpy(targetbuf, CONST_CSTR_STRING(target), len); + targetfile = fopen(targetbuf, "wb"); + if (targetfile == NULL) { + Obj prec = NEW_PREC(2); + SET_LEN_PREC(prec, 2); + SET_RNAM_PREC(prec, 1, RNamName("success")); + SET_ELM_PREC(prec, 1, False); + SET_RNAM_PREC(prec, 2, RNamName("error")); + SET_ELM_PREC(prec, 2, MakeImmString("cannot open target file")); + CHANGED_BAG(prec); + free(targetbuf); + return prec; + } + } + res = curl_global_init(CURL_GLOBAL_DEFAULT); if (res != 0) { ErrorMayQuit("CURL_REQUEST: failed to initialize libcurl (error %d)", @@ -82,8 +115,14 @@ Obj FuncCURL_REQUEST(Obj self, Obj input_list) curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, errbuf); curl_easy_setopt(curl, CURLOPT_URL, urlbuf); - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_string); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, in_string); + if (targetfile != NULL) { + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_file); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, targetfile); + } + else { + curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_string); + curl_easy_setopt(curl, CURLOPT_WRITEDATA, in_string); + } curl_easy_setopt(curl, CURLOPT_TCP_NODELAY, 1L); curl_easy_setopt(curl, CURLOPT_USERAGENT, "curlInterface/GAP package"); @@ -171,6 +210,18 @@ Obj FuncCURL_REQUEST(Obj self, Obj input_list) curl_global_cleanup(); free(typebuf); + if (targetfile != NULL) { + if (fclose(targetfile) != 0 && errorstring == 0) + errorstring = MakeImmString("cannot write target file"); + // Do not leave a partial or empty file behind after a failure; a + // caller that tests whether the file exists must not be told yes. + if (errorstring) + remove(targetbuf); + free(targetbuf); + } + + // With a target file there is no body to hand back, so the result record + // has just 'success', or 'success' and 'error'. Obj prec = NEW_PREC(2); SET_LEN_PREC(prec, 2); SET_RNAM_PREC(prec, 1, RNamName("success")); @@ -179,6 +230,10 @@ Obj FuncCURL_REQUEST(Obj self, Obj input_list) SET_RNAM_PREC(prec, 2, RNamName("error")); SET_ELM_PREC(prec, 2, errorstring); } + else if (targetfile != NULL) { + SET_LEN_PREC(prec, 1); + SET_ELM_PREC(prec, 1, True); + } else { SET_ELM_PREC(prec, 1, True); SET_RNAM_PREC(prec, 2, RNamName("result")); @@ -195,8 +250,8 @@ Obj FuncCURL_VERSION(Obj self) // Table of functions to export static StructGVarFunc GVarFuncs[] = { - GVAR_FUNC(CURL_REQUEST, 8, - "url, type, out_string, verifyCert, verbose, followRedirect, failOnError, maxTime"), + GVAR_FUNC(CURL_REQUEST, 9, + "url, type, out_string, verifyCert, verbose, followRedirect, failOnError, maxTime, target"), GVAR_FUNC(CURL_VERSION, 0, ""), { 0 } }; diff --git a/tst/basic.tst b/tst/basic.tst index 32f2830..ad38357 100644 --- a/tst/basic.tst +++ b/tst/basic.tst @@ -1,4 +1,4 @@ -#@local r, url, postString, requestType, server, baseurl +#@local r, url, postString, requestType, server, baseurl, file gap> LoadPackage( "curlInterface", false ); true gap> LoadPackage( "io", false ); @@ -110,4 +110,46 @@ gap> DownloadURL( url, rec( maxTime := 1 ) ).success; false gap> DownloadURL( url, rec( maxTime := 5 ) ).result; "download test response\n" + +# Downloading to a file +gap> file := Filename( DirectoryTemporary(), "target" );; +gap> r := DownloadURL( Concatenation( baseurl, "/success" ), +> rec( target := file ) );; +gap> r.success; +true + +# with a target there is no body to hand back +gap> RecNames( r ); +[ "success" ] +gap> StringFile( file ); +"download test response\n" + +# a failed request must not leave the file behind +gap> RemoveFile( file );; +gap> r := DownloadURL( Concatenation( baseurl, "/missing" ), +> rec( target := file, failOnError := true ) );; +gap> r.success; +false +gap> IsExistingFile( file ); +false + +# nor after the connection drops mid-transfer +gap> r := DownloadURL( Concatenation( baseurl, "/disconnect" ), +> rec( target := file ) );; +gap> r.success; +false +gap> IsExistingFile( file ); +false + +# a target that cannot be opened is reported, not fatal +gap> r := DownloadURL( Concatenation( baseurl, "/success" ), +> rec( target := "/no/such/directory/target" ) );; +gap> r.success; +false +gap> r.error; +"cannot open target file" + +# argument checking +gap> DownloadURL( baseurl, rec( target := 42 ) ); +Error, CurlRequest: .target must be a string or false gap> CURLINTERFACE_StopHTTPTestServer( server );; diff --git a/tst/errors.tst b/tst/errors.tst index 54ec418..f613079 100644 --- a/tst/errors.tst +++ b/tst/errors.tst @@ -63,4 +63,4 @@ Error, CurlRequest: .maxTime must be a non-negative integer # number of arguments gap> CURL_REQUEST(); -Error, Function: number of arguments must be 8 (not 0) +Error, Function: number of arguments must be 9 (not 0)