Download: never leave the target file behind after a failure

doc/download.xml

@@ -68,6 +68,8 @@ The following components are supported.
   that is a local filename,
   and the function writes the downloaded contents to this file;
   the returned record does not have a <C>result</C> component in this case.
+  <P/>
+  If the download fails then this file is not left behind.
 </Item>
 <Mark><C>verifyCert</C></Mark>
 <Item>

lib/download.gi

@@ -270,6 +270,13 @@ InstallMethod( Download,
         if res.success = true then
           return res;
         fi;
+        # A failed method may have left a partial or bogus target file behind.
+        # Remove it here, so that the guarantee holds for every method,
+        # including ones added to 'Download_Methods' from outside.
+        if IsBound( opt.target ) and IsString( opt.target ) and
+           IsExistingFile( opt.target ) then
+          RemoveFile( opt.target );
+        fi;
         Info( InfoUtils, 2, "Download method ", r.name, " failed with\n",
               "#I    ", res.error );
         Add( errors, Concatenation( r.name, ": ", res.error ) );

tst/download.tst

@@ -139,6 +139,18 @@ gap> res1:= Download( url, rec( maxTime:= 5 ) );;
 gap> res1.success = true;
 true
 
+##  A failed download must not leave the target file behind, whichever
+##  method was tried.  'Download' is where that is guaranteed: an individual
+##  method may well leave a partial file, and one of them has to, since a
+##  resuming method needs what the previous attempt wrote.
+gap> file:= Filename( DirectoryTemporary(), "target" );;
+gap> res1:= Download( Concatenation( baseurl, "/missing" ),
+>                     rec( target:= file ) );;
+gap> res1.success = false;
+true
+gap> IsExistingFile( file );
+false
+
 ##  test errors and redirects
 gap> res1:= Download( Concatenation( baseurl, "/missing" ) );;
 gap> res1.success = false;