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..e52cc6e 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,58 @@ 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( 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 +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 +70,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 +131,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( "https://httpbun.com/delay/3", rec( maxTime:= 5 ) );; +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( 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 );