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 );