From 4ef18fb7b920b4345e42bc0afe589cf5fd6bc073 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 17 Jul 2026 19:29:55 +0200 Subject: [PATCH 1/2] Use local server for download tests Replace live network assertions with a forked HTTP server provided by the IO test dependency. Teach the SingleHTTPRequest adapter to handle explicit ports so every backend can use the ephemeral local address. Keep the public HTTPS transcript as non-executable documentation. Co-authored-by: Codex --- PackageInfo.g | 1 + doc/download.xml | 4 +-- lib/download.gi | 69 ++++++++++++++++++++++++++++++----- tst/download.tst | 60 ++++++++++++++++++++++++------- tst/http-server.g | 92 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 202 insertions(+), 24 deletions(-) create mode 100644 tst/http-server.g diff --git a/PackageInfo.g b/PackageInfo.g index 9544bb0..c83b33a 100644 --- a/PackageInfo.g +++ b/PackageInfo.g @@ -126,6 +126,7 @@ Dependencies := rec( GAP := ">=4.10.1", NeededOtherPackages := [ ], SuggestedOtherPackages := [ [ "curlInterface", ">= 2.3.0" ] ], + TestPackages := [ [ "io", ">= 4.7.0" ] ], ExternalConditions := [ ] ), diff --git a/doc/download.xml b/doc/download.xml index f175141..def4273 100644 --- a/doc/download.xml +++ b/doc/download.xml @@ -82,7 +82,7 @@ The following components are supported.

- url:= "https://www.gap-system.org/index.html";; gap> res1:= Download( url );; gap> res1.success; @@ -94,7 +94,7 @@ gap> res2.success; false gap> IsBound( res2.error ) and IsString( res2.error ); true -]]> +]]> diff --git a/lib/download.gi b/lib/download.gi index 21ef3a1..1e01b2e 100644 --- a/lib/download.gi +++ b/lib/download.gi @@ -55,7 +55,7 @@ Add( Download_Methods, rec( name:= "via SingleHTTPRequest (from the IO package)", isAvailable:= {} -> IsBoundGlobal( "SingleHTTPRequest" ), download:= function( url, opt ) - local rurl, pos, domain, uri, res; + local rurl, pos, authority, domain, port, portstr, uri, fragment, res; if not StartsWith( url, "http://" ) then return rec( success:= false, error:= "protocol is not http" ); @@ -63,16 +63,67 @@ Add( Download_Methods, rec( return rec( success:= false, error:= "no support for given timeout" ); fi; - rurl:= ReplacedString( url, "http://", "" ); - pos:= Position( rurl, '/' ); - domain:= rurl{ [ 1 .. pos-1 ] }; - uri:= rurl{ [ pos .. Length( rurl ) ] }; + # Split the URL after 'http://' into the authority and HTTP request target. + # A query without an explicit path gets the default path '/'. + rurl:= url{ [ 8 .. Length( url ) ] }; + pos:= PositionProperty( rurl, c -> c in "/?#" ); + if pos = fail then + authority:= rurl; + uri:= "/"; + else + authority:= rurl{ [ 1 .. pos-1 ] }; + uri:= rurl{ [ pos .. Length( rurl ) ] }; + if StartsWith( uri, "?" ) then + uri:= Concatenation( "/", uri ); + elif StartsWith( uri, "#" ) then + uri:= "/"; + fi; + fi; + + # A fragment is interpreted by the client and must not be sent to the + # server as part of the request target. + fragment:= Position( uri, '#' ); + if fragment <> fail then + uri:= uri{ [ 1 .. fragment-1 ] }; + fi; + + # Separate the optional port from the host. IO's HTTP client uses IPv4 + # sockets, thus bracketed IPv6 addresses are not supported here. + domain:= authority; + port:= 80; + portstr:= fail; + if Length( authority ) = 0 or Position( authority, '@' ) <> fail then + return rec( success:= false, error:= "invalid URL authority" ); + elif authority[1] = '[' then + return rec( success:= false, + error:= "IPv6 addresses are not supported" ); + else + pos:= Position( authority, ':' ); + if pos <> fail then + if Position( authority, ':', pos ) <> fail then + return rec( success:= false, error:= "invalid URL authority" ); + fi; + domain:= authority{ [ 1 .. pos-1 ] }; + portstr:= authority{ [ pos+1 .. Length( authority ) ] }; + fi; + fi; + if Length( domain ) = 0 then + return rec( success:= false, error:= "invalid URL authority" ); + elif portstr <> fail then + port:= Int( portstr ); + if not IsPosInt( port ) or port > 65535 then + return rec( success:= false, error:= "invalid URL port" ); + fi; + fi; + + # Preserve the complete authority, including brackets and a non-default + # port, in the HTTP Host header. if IsBound( opt.target ) and IsString( opt.target ) then - res:= ValueGlobal( "SingleHTTPRequest" )( domain, 80, "GET", uri, - rec(), false, opt.target ); + res:= ValueGlobal( "SingleHTTPRequest" )( domain, port, "GET", uri, + rec( Host:= authority ), false, opt.target ); else - res:= ValueGlobal( "SingleHTTPRequest" )( domain, 80, "GET", uri, - rec(), false, false ); + res:= ValueGlobal( "SingleHTTPRequest" )( domain, port, "GET", uri, + rec( Host:= authority ), false, false ); fi; if res.statuscode = 0 then return rec( success:= false, diff --git a/tst/download.tst b/tst/download.tst index cb19c10..0a2a5fd 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 +#@local meths, i, urls, pair, url, expected, res1, good1, n, file, res2, good2, contents, r, res3, good3, bad, server, baseurl, iometh ############################################################################ ## #W download.tst Utils Package Thomas Breuer @@ -11,21 +11,42 @@ gap> START_TEST( "download.tst" ); gap> ReadPackage( "utils", "tst/loadall.g" );; gap> UtilsLoadingComplete; true +gap> LoadPackage( "io", false ); +true +gap> ReadPackage( "utils", "tst/http-server.g" );; +gap> server:= UTILS_StartHTTPTestServer();; +gap> baseurl:= Concatenation( "http://127.0.0.1:", +> String( server.port ) );; ## Test the available Download methods gap> meths:= List( Filtered( Download_Methods, r -> r.isAvailable() ), > ShallowCopy );; +gap> iometh:= First( meths, +> r -> StartsWith( r.name, "via SingleHTTPRequest" ) );; gap> for i in [ 1 .. Length( meths ) ] do > meths[i].position:= String( i ); > od; -gap> urls:= [ # a http url that gets redirected to https -> [ "http://www.gap-system.org/index.html", true ], -> # a http url that works as such -> [ "http://www.math.rwth-aachen.de/index.html", true ], -> # a https url that exists -> [ "https://www.gap-system.org/index.html", true ], -> # a https url that does not exist -> [ "https://www.gap-system.org/indexxxxx.html", false ], + +## Test URL parsing in the SingleHTTPRequest based method. +gap> res1:= iometh.download( Concatenation( baseurl, "?a=b#fragment" ), +> rec() );; +gap> res1.success; +true +gap> res1:= iometh.download( "http://127.0.0.1:not-a-port/", rec() );; +gap> res1.success = false; +true +gap> res1:= iometh.download( "http://127.0.0.1:65536/", rec() );; +gap> res1.success = false; +true +gap> res1:= iometh.download( +> Concatenation( "http://[::1]:", String( server.port ), "/" ), +> rec() );; +gap> res1.success = false; +true +gap> res1.error; +"IPv6 addresses are not supported" +gap> urls:= [ # an http url that works as such +> [ Concatenation( baseurl, "/success" ), true ], > ];; ## The problem is that the methods do not behave consistently @@ -33,13 +54,15 @@ gap> urls:= [ # a http url that gets redirected to https ## (Well, they even do not agree what failure means.) ## The test results depend on which methods are available at runtime, ## which makes them useless as automatic tests. -## Thus we test only working http and https urls. -gap> urls:= urls{ [ 2, 3 ] };; +## Thus we test only a working http url. gap> for pair in urls do > url:= pair[1]; > expected:= pair[2]; > res1:= List( meths, r -> [ r.download( url, rec() ), r.position ] );; > good1:= Filtered( res1, r -> r[1].success = true );; +> if expected = true and Length( good1 ) <> Length( meths ) then +> Print( "failure for url ", url, "\n" ); +> fi; > if expected = false and Length( good1 ) > 0 then > Print( "success for url ", url, "?\n" ); > fi; @@ -92,12 +115,23 @@ gap> for pair in urls do > od; ## test timeout -gap> res1:= Download( "https://httpbun.com/delay/3", rec( maxTime:= 1 ) );; +gap> url:= Concatenation( baseurl, "/delay/3" );; +gap> res1:= Download( url, rec( maxTime:= 1 ) );; +gap> res1.success = false; +true +gap> res1:= Download( url, rec( maxTime:= 5 ) );; +gap> res1.success = true; +true + +## test errors and redirects +gap> res1:= Download( Concatenation( baseurl, "/missing" ) );; gap> res1.success = false; true -gap> res1:= Download( "https://httpbun.com/delay/3", rec( maxTime:= 5 ) );; +gap> res1:= Download( Concatenation( baseurl, "/redirect" ) );; gap> res1.success = true; true +gap> UTILS_StopHTTPTestServer( server );; + ## gap> STOP_TEST( "download.tst" ); diff --git a/tst/http-server.g b/tst/http-server.g new file mode 100644 index 0000000..40a532a --- /dev/null +++ b/tst/http-server.g @@ -0,0 +1,92 @@ +############################################################################# +## +## A small HTTP server for the download tests. +## + +BindGlobal( "UTILS_HandleHTTPTestRequest", function( listener, socket ) + local connection, line, parts, uri, body, status, location; + + IO_close( listener ); + connection:= IO_WrapFD( socket, IO.DefaultBufSize, IO.DefaultBufSize ); + line:= IO_ReadLine( connection ); + parts:= SplitString( line, " \r\n" ); + if Length( parts ) < 2 then + IO_Close( connection ); + IO_exit( 1 ); + fi; + uri:= parts[2]; + + repeat + line:= IO_ReadLine( connection ); + until line = fail or line = "" or line = "\n" or line = "\r\n"; + + body:= "download test response\n"; + status:= "200 OK"; + location:= ""; + if StartsWith( uri, "/delay/3" ) then + Sleep( 3 ); + elif StartsWith( uri, "/missing" ) then + body:= "not found\n"; + status:= "404 Not Found"; + elif StartsWith( uri, "/redirect" ) then + body:= ""; + status:= "302 Found"; + location:= "Location: /success\r\n"; + fi; + + IO_Write( connection, + "HTTP/1.1 ", status, "\r\n", + location, + "Content-Type: text/plain\r\n", + "Content-Length: ", String( Length( body ) ), "\r\n", + "Connection: close\r\n\r\n", + body ); + IO_Flush( connection ); + IO_Close( connection ); + IO_exit( 0 ); +end ); + +BindGlobal( "UTILS_StartHTTPTestServer", function() + local listener, address, port, pid, socket, handler; + + listener:= IO_socket( IO.PF_INET, IO.SOCK_STREAM, "tcp" ); + if listener = fail or + IO_bind( listener, IO_MakeIPAddressPort( "127.0.0.1", 0 ) ) = fail or + IO_listen( listener, 8 ) <> true then + Error( "cannot start the HTTP test server" ); + fi; + address:= IO_getsockname( listener ); + port:= 256 * INT_CHAR( address[3] ) + INT_CHAR( address[4] ); + + pid:= IO_fork(); + if pid = 0 then + while true do + socket:= IO_accept( listener, + IO_MakeIPAddressPort( "0.0.0.0", 0 ) ); + if socket = fail then + IO_exit( 0 ); + fi; + handler:= IO_fork(); + if handler = 0 then + UTILS_HandleHTTPTestRequest( listener, socket ); + elif handler < 0 then + IO_close( socket ); + IO_exit( 1 ); + else + IO_close( socket ); + IO_IgnorePid( handler ); + fi; + od; + elif pid < 0 then + IO_close( listener ); + Error( "cannot fork the HTTP test server" ); + fi; + + IO_close( listener ); + return rec( pid:= pid, port:= port ); +end ); + +BindGlobal( "UTILS_StopHTTPTestServer", function( server ) + IO_kill( server.pid, IO.SIGTERM ); + IO_WaitPid( server.pid, true ); +end ); From 415435a47ce9566a99095dd00ecef131dc34f8cf Mon Sep 17 00:00:00 2001 From: Max Horn Date: Fri, 17 Jul 2026 23:58:45 +0200 Subject: [PATCH 2/2] Cover HTTP URL parsing branches Exercise URLs without paths, authority-only fragments, and malformed authorities in the SingleHTTPRequest adapter tests. Co-authored-by: Codex --- tst/download.tst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tst/download.tst b/tst/download.tst index 0a2a5fd..e52cc6e 100644 --- a/tst/download.tst +++ b/tst/download.tst @@ -32,6 +32,22 @@ gap> res1:= iometh.download( Concatenation( baseurl, "?a=b#fragment" ), > rec() );; gap> res1.success; true +gap> res1:= iometh.download( baseurl, rec() );; +gap> res1.success; +true +gap> res1:= iometh.download( Concatenation( baseurl, "#fragment" ), +> rec() );; +gap> res1.success; +true +gap> res1:= iometh.download( "http:///missing-authority", rec() );; +gap> res1.error; +"invalid URL authority" +gap> res1:= iometh.download( "http://127.0.0.1:80:90/", rec() );; +gap> res1.error; +"invalid URL authority" +gap> res1:= iometh.download( "http://:80/", rec() );; +gap> res1.error; +"invalid URL authority" gap> res1:= iometh.download( "http://127.0.0.1:not-a-port/", rec() );; gap> res1.success = false; true