Use local server for download tests

PackageInfo.g

@@ -126,6 +126,7 @@ Dependencies := rec(
   GAP := ">=4.10.1",
   NeededOtherPackages := [ ], 
   SuggestedOtherPackages := [ [ "curlInterface", ">= 2.3.0" ] ],
+  TestPackages := [ [ "io", ">= 4.7.0" ] ],
   ExternalConditions := [ ]
 ),
 

doc/download.xml

@@ -82,7 +82,7 @@ The following components are supported.
 </Item>
 </List>
 <P/>
-<Example><![CDATA[
+<Log><![CDATA[
 gap> 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
-]]></Example>
+]]></Log>
 </Description>
 </ManSection>
 

lib/download.gi

@@ -55,24 +55,75 @@ 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" );
     elif IsBound( opt.maxTime ) and opt.maxTime <> 0 then
       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,

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,35 +11,74 @@ 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
 ##  in the case of failure.
 ##  (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" );

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