Do not create file before recv body when download (#3381)

php_swoole_cxx.h

@@ -168,14 +168,19 @@ class string
         return sw_likely(len() > 0) ? estrndup(val(), len()) : nullptr;
     }
 
-    ~string()
+    void release()
     {
         if (str)
         {
             zend_string_release(str);
         }
     }
 
+    ~string()
+    {
+        release();
+    }
+
 private:
     zend_string *str;
 };

swoole_http_client_coro.cc

@@ -88,7 +88,9 @@ class http_client
 #ifdef SW_HAVE_ZLIB
     bool websocket_compression = false; // allow to compress websocket messages
 #endif
-    int  download_file_fd = 0;          // save http response to file
+    int download_file_fd = -1;          // save http response to file
+    zend::string download_file_name;    // unlink the file on error
+    zend_long download_offset = 0;
     bool has_upload_files = false;
 
     /* safety zval */
@@ -515,8 +517,37 @@ static int http_parser_on_body(swoole_http_parser *parser, const char *at, size_
             return -1;
         }
     }
-    if (http->download_file_fd > 0 && http->body->length > 0)
+    if (http->download_file_name.get() && http->body->length > 0)
     {
+        if (http->download_file_fd < 0)
+        {
+            char *download_file_name = http->download_file_name.val();
+            int fd = ::open(download_file_name, O_CREAT | O_WRONLY, 0664);
+            if (fd < 0)
+            {
+                swSysWarn("open(%s, O_CREAT | O_WRONLY) failed", download_file_name);
+                return false;
+            }
+            if (http->download_offset == 0)
+            {
+                if (::ftruncate(fd, 0) < 0)
+                {
+                    swSysWarn("ftruncate(%s) failed", download_file_name);
+                    ::close(fd);
+                    return false;
+                }
+            }
+            else
+            {
+                if (::lseek(fd, http->download_offset, SEEK_SET) < 0)
+                {
+                    swSysWarn("fseek(%s, %jd) failed", download_file_name, (intmax_t) http->download_offset);
+                    ::close(fd);
+                    return false;
+                }
+            }
+            http->download_file_fd = fd;
+        }
         if (swoole_coroutine_write(http->download_file_fd, SW_STRINGL(http->body)) != (ssize_t) http->body->length)
         {
             return -1;
@@ -543,6 +574,10 @@ static int http_parser_on_message_complete(swoole_http_parser *parser)
     {
         zend_update_property_stringl(swoole_http_client_coro_ce, zobject, ZEND_STRL("body"), SW_STRINGL(http->body));
     }
+    else
+    {
+        http->download_file_name.release();
+    }
 
     if (parser->upgrade)
     {
@@ -919,36 +954,8 @@ bool http_client::send()
     // ============ download ============
     if (z_download_file)
     {
-        zend::string str_download_file(z_download_file);
-        char *download_file_name = str_download_file.val();
-        zval *z_download_offset = sw_zend_read_property_ex(swoole_http_client_coro_ce, zobject, SW_ZSTR_KNOWN(SW_ZEND_STR_DOWNLOAD_OFFSET), 0);
-        off_t download_offset = zval_get_long(z_download_offset);
-
-        int fd = ::open(download_file_name, O_CREAT | O_WRONLY, 0664);
-        if (fd < 0)
-        {
-            swSysWarn("open(%s, O_CREAT | O_WRONLY) failed", download_file_name);
-            return false;
-        }
-        if (download_offset == 0)
-        {
-            if (ftruncate(fd, 0) < 0)
-            {
-                swSysWarn("ftruncate(%s) failed", download_file_name);
-                ::close(fd);
-                return false;
-            }
-        }
-        else
-        {
-            if (lseek(fd, download_offset, SEEK_SET) < 0)
-            {
-                swSysWarn("fseek(%s, %jd) failed", download_file_name, (intmax_t) download_offset);
-                ::close(fd);
-                return false;
-            }
-        }
-        download_file_fd = fd;
+        download_file_name = z_download_file;
+        download_offset = zval_get_long(sw_zend_read_property_ex(swoole_http_client_coro_ce, zobject, SW_ZSTR_KNOWN(SW_ZEND_STR_DOWNLOAD_OFFSET), 0));
     }
 
     // ============ method ============
@@ -1663,10 +1670,12 @@ void http_client::reset()
     {
         zend_update_property_null(swoole_http_client_coro_ce, zobject, ZEND_STRL("uploadFiles"));
     }
-    if (download_file_fd > 0)
+    if (download_file_fd >= 0)
     {
         ::close(download_file_fd);
-        download_file_fd = 0;
+        download_file_fd = -1;
+        download_file_name.release();
+        download_offset = 0;
         zend_update_property_null(swoole_http_client_coro_ce, zobject, ZEND_STRL("downloadFile"));
         zend_update_property_long(swoole_http_client_coro_ce, zobject, ZEND_STRL("downloadOffset"), 0);
     }

tests/swoole_http_client_coro/download_failed.phpt

@@ -0,0 +1,56 @@
+--TEST--
+swoole_http_client_coro: http download io failure
+--SKIPIF--
+<?php
+require __DIR__ . '/../include/skipif.inc';
+?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+
+const FILE = '/tmp/download.html';
+
+@unlink(FILE);
+Co\run(function () {
+    $server = new Co\Socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
+    Assert::assert($server->bind('127.0.0.1', 0));
+    Assert::assert($server->listen());
+    $oort = $server->getsockname()['port'];
+    go(function () use ($server) {
+        $client = $server->accept();
+        while ($client->recv(1)) {
+            CO::sleep(0.01);
+        }
+        $server->close();
+    });
+    $cli = new Swoole\Coroutine\Http\Client('127.0.0.1', $oort);
+    $cli->set(['timeout' => 0.1]);
+    Assert::false($cli->download('/get', FILE));
+    Assert::false(file_exists(FILE));
+    echo "OK\n";
+});
+Co\run(function () {
+    $server = new Co\Socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
+    Assert::assert($server->bind('127.0.0.1', 0));
+    Assert::assert($server->listen());
+    $oort = $server->getsockname()['port'];
+    go(function () use ($server) {
+        $client = $server->accept();
+        $client->send("HTTP/1.1 200 OK\r\nContent-Length: 99999\r\n\r\n");
+        while ($client->send('a')) {
+            CO::sleep(0.001);
+        }
+        $server->close();
+    });
+    $cli = new Swoole\Coroutine\Http\Client('127.0.0.1', $oort);
+    $cli->set(['timeout' => 0.1]);
+    Assert::false($cli->download('/get', FILE));
+    Assert::true(file_exists(FILE));
+    echo "OK\n";
+});
+@unlink(FILE);
+
+?>
+--EXPECT--
+OK
+OK