Fix bug #5547 (#5586)

swoole · Nov 26, 2024 · 3c7d3f5 · 3c7d3f5
1 parent 5942dc5
commit 3c7d3f5
Show file tree

Hide file tree

Showing 12 changed files with 107 additions and 81 deletions.
diff --git a/core-tests/src/network/dns.cpp b/core-tests/src/network/dns.cpp
@@ -74,23 +74,9 @@ TEST(dns, cancel) {
 }
 
 TEST(dns, getaddrinfo) {
-    char buf[1024] = {};
-    swoole::network::GetaddrinfoRequest req = {};
-    req.hostname = "www.baidu.com";
-    req.family = AF_INET;
-    req.socktype = SOCK_STREAM;
-    req.protocol = 0;
-    req.service = nullptr;
-    req.result = buf;
+    swoole::GetaddrinfoRequest req("www.baidu.com", AF_INET, SOCK_STREAM, 0, "");
     ASSERT_EQ(swoole::network::getaddrinfo(&req), 0);
     ASSERT_GT(req.count, 0);
-
-    vector<string> ip_list;
-    req.parse_result(ip_list);
-
-    for (auto &ip : ip_list) {
-        ASSERT_TRUE(swoole::network::Address::verify_ip(AF_INET, ip));
-    }
 }
 
 TEST(dns, load_resolv_conf) {

diff --git a/include/swoole_async.h b/include/swoole_async.h
@@ -34,13 +34,17 @@ enum AsyncFlag {
     SW_AIO_EOF = 1u << 2,
 };
 
+struct AsyncRequest {
+    virtual ~AsyncRequest() = default;
+};
+
 struct AsyncEvent {
     size_t task_id;
     uint8_t canceled;
     /**
      * input & output
      */
-    void *data;
+    std::shared_ptr<AsyncRequest> data;
     /**
      * output
      */
@@ -60,22 +64,44 @@ struct AsyncEvent {
     }
 };
 
-struct GethostbynameRequest {
-    const char *name;
+struct GethostbynameRequest : public AsyncRequest {
+    std::string name;
     int family;
     char *addr;
     size_t addr_len;
 
-    GethostbynameRequest(const char *_name, int _family) : name(_name), family(_family) {
+    GethostbynameRequest(std::string _name, int _family) : name(std::move(_name)), family(_family) {
         addr_len = _family == AF_INET6 ? INET6_ADDRSTRLEN : INET_ADDRSTRLEN;
         addr = new char[addr_len];
     }
 
-    ~GethostbynameRequest() {
+    ~GethostbynameRequest() override {
         delete[] addr;
     }
 };
 
+struct GetaddrinfoRequest : public AsyncRequest {
+    std::string hostname;
+    std::string service;
+    int family;
+    int socktype;
+    int protocol;
+    int error;
+    std::vector<struct sockaddr_in6> results;
+    int count;
+    void parse_result(std::vector<std::string> &retval);
+    GetaddrinfoRequest(std::string _hostname, int _family, int _socktype, int _protocol, std::string _service)
+        : hostname(std::move(_hostname)),
+          service(std::move(_service)) {
+        family =_family;
+        socktype =_socktype;
+        protocol =_protocol;
+        count = 0;
+        error = 0;
+    }
+    ~GetaddrinfoRequest() override = default;
+};
+
 class AsyncThreads {
   public:
     bool schedule = false;

diff --git a/include/swoole_socket.h b/include/swoole_socket.h
@@ -64,20 +64,9 @@ enum {
 };
 
 namespace swoole {
-namespace network {
+struct GetaddrinfoRequest;
 
-struct GetaddrinfoRequest {
-    const char *hostname;
-    const char *service;
-    int family;
-    int socktype;
-    int protocol;
-    int error;
-    void *result;
-    int count;
-
-    void parse_result(std::vector<std::string> &retval);
-};
+namespace network {
 
 struct SendfileTask {
     off_t offset;

diff --git a/src/coroutine/system.cc b/src/coroutine/system.cc
@@ -137,8 +137,8 @@ ssize_t System::write_file(const char *file, char *buf, size_t length, bool lock
 
 std::string gethostbyname_impl_with_async(const std::string &hostname, int domain, double timeout) {
     AsyncEvent ev{};
-    GethostbynameRequest dns_request(hostname.c_str(), domain);
-    ev.data = &dns_request;
+    auto req = new GethostbynameRequest(hostname, domain);
+    ev.data = std::shared_ptr<AsyncRequest>(req);
     ev.retval = 1;
 
     coroutine::async(async::handler_gethostbyname, ev, timeout);
@@ -150,7 +150,7 @@ std::string gethostbyname_impl_with_async(const std::string &hostname, int domai
         swoole_set_last_error(ev.error);
         return "";
     } else {
-        std::string addr(dns_request.addr);
+        std::string addr(req->addr);
         return addr;
     }
 }
@@ -199,30 +199,20 @@ std::vector<std::string> System::getaddrinfo(
     assert(family == AF_INET || family == AF_INET6);
 
     AsyncEvent ev{};
-    network::GetaddrinfoRequest req{};
-
-    ev.data = &req;
-
-    struct sockaddr_in6 result_buffer[SW_DNS_HOST_BUFFER_SIZE];
-
-    req.hostname = hostname.c_str();
-    req.family = family;
-    req.socktype = socktype;
-    req.protocol = protocol;
-    req.service = service.empty() ? nullptr : service.c_str();
-    req.result = result_buffer;
+    auto req = new GetaddrinfoRequest(hostname, family, socktype, protocol, service);
+    ev.data = std::shared_ptr<AsyncRequest>(req);
 
     coroutine::async(async::handler_getaddrinfo, ev, timeout);
 
     std::vector<std::string> retval;
 
-    if (ev.retval == -1 || req.error != 0) {
+    if (ev.retval == -1 || req->error != 0) {
         if (ev.error == SW_ERROR_AIO_TIMEOUT) {
             ev.error = SW_ERROR_DNSLOOKUP_RESOLVE_TIMEOUT;
         }
         swoole_set_last_error(ev.error);
     } else {
-        req.parse_result(retval);
+        req->parse_result(retval);
     }
 
     return retval;

diff --git a/src/network/client.cc b/src/network/client.cc
@@ -615,14 +615,13 @@ static int Client_tcp_connect_async(Client *cli, const char *host, int port, dou
 
     if (cli->wait_dns) {
         AsyncEvent ev{};
-        auto dns_request = new GethostbynameRequest(cli->server_host, cli->_sock_domain);
-        ev.data = dns_request;
+        auto req = new GethostbynameRequest(cli->server_host, cli->_sock_domain);
+        ev.data = std::shared_ptr<AsyncRequest>(req);;
         ev.object = cli;
         ev.handler = async::handler_gethostbyname;
         ev.callback = Client_onResolveCompleted;
 
         if (swoole::async::dispatch(&ev) == nullptr) {
-            delete dns_request;
             return SW_ERR;
         } else {
             return SW_OK;
@@ -1112,17 +1111,13 @@ static void Client_onTimeout(Timer *timer, TimerNode *tnode) {
 }
 
 static void Client_onResolveCompleted(AsyncEvent *event) {
-    auto dns_request = (GethostbynameRequest *) event->data;
-    if (event->canceled) {
-        delete dns_request;
-        return;
-    }
+    GethostbynameRequest *req = dynamic_cast<GethostbynameRequest *>(event->data.get());
 
     Client *cli = (Client *) event->object;
     cli->wait_dns = 0;
 
     if (event->error == 0) {
-        Client_tcp_connect_async(cli, dns_request->addr, cli->server_port, cli->timeout, 1);
+        Client_tcp_connect_async(cli, req->addr, cli->server_port, cli->timeout, 1);
     } else {
         swoole_set_last_error(SW_ERROR_DNSLOOKUP_RESOLVE_FAILED);
         cli->socket->removed = 1;
@@ -1131,7 +1126,6 @@ static void Client_onResolveCompleted(AsyncEvent *event) {
             cli->onError(cli);
         }
     }
-    delete dns_request;
 }
 
 static int Client_onWrite(Reactor *reactor, Event *event) {

diff --git a/src/network/dns.cc b/src/network/dns.cc
@@ -344,7 +344,7 @@ std::vector<std::string> dns_lookup_impl_with_socket(const char *domain, int fam
         temp = &packet[steps];
         j = 0;
         while (*temp != 0) {
-            if ((uchar)(*temp) == 0xc0) {
+            if ((uchar) (*temp) == 0xc0) {
                 ++temp;
                 temp = &packet[(uint8_t) *temp];
             } else {
@@ -373,7 +373,7 @@ std::vector<std::string> dns_lookup_impl_with_socket(const char *domain, int fam
             temp = &packet[steps];
             j = 0;
             while (*temp != 0) {
-                if ((uchar)(*temp) == 0xc0) {
+                if ((uchar) (*temp) == 0xc0) {
                     ++temp;
                     temp = &packet[(uint8_t) *temp];
                 } else {
@@ -767,36 +767,40 @@ int getaddrinfo(GetaddrinfoRequest *req) {
     hints.ai_socktype = req->socktype;
     hints.ai_protocol = req->protocol;
 
-    int ret = ::getaddrinfo(req->hostname, req->service, &hints, &result);
+    int ret = ::getaddrinfo(req->hostname.c_str(), req->service.c_str(), &hints, &result);
     if (ret != 0) {
         req->error = ret;
         return SW_ERR;
     }
 
-    void *buffer = req->result;
     int i = 0;
-    for (ptr = result; ptr != nullptr; ptr = ptr->ai_next) {
+    for (ptr = result; ptr != nullptr; ptr = ptr->ai_next, i++) {
+    }
+    req->count = SW_MIN(i, SW_DNS_HOST_BUFFER_SIZE);
+    req->results.resize(req->count);
+
+    for (ptr = result, i = 0; ptr != nullptr; ptr = ptr->ai_next, i++) {
         switch (ptr->ai_family) {
         case AF_INET:
-            memcpy((char *) buffer + (i * sizeof(struct sockaddr_in)), ptr->ai_addr, sizeof(struct sockaddr_in));
+            memcpy(&req->results[i], ptr->ai_addr, sizeof(struct sockaddr_in));
             break;
         case AF_INET6:
-            memcpy((char *) buffer + (i * sizeof(struct sockaddr_in6)), ptr->ai_addr, sizeof(struct sockaddr_in6));
+            memcpy(&req->results[i], ptr->ai_addr, sizeof(struct sockaddr_in6));
             break;
         default:
             swoole_warning("unknown socket family[%d]", ptr->ai_family);
             break;
         }
-        i++;
         if (i == SW_DNS_HOST_BUFFER_SIZE) {
             break;
         }
     }
     ::freeaddrinfo(result);
     req->error = 0;
-    req->count = i;
+
     return SW_OK;
 }
+}  // namespace network
 
 void GetaddrinfoRequest::parse_result(std::vector<std::string> &retval) {
     struct sockaddr_in *addr_v4;
@@ -805,18 +809,17 @@ void GetaddrinfoRequest::parse_result(std::vector<std::string> &retval) {
     char tmp[INET6_ADDRSTRLEN];
     const char *r;
 
-    for (int i = 0; i < count; i++) {
+    for (auto &addr : results) {
         if (family == AF_INET) {
-            addr_v4 = (struct sockaddr_in *) ((char *) result + (i * sizeof(struct sockaddr_in)));
+            addr_v4 = (struct sockaddr_in *) &addr;
             r = inet_ntop(AF_INET, (const void *) &addr_v4->sin_addr, tmp, sizeof(tmp));
         } else {
-            addr_v6 = (struct sockaddr_in6 *) ((char *) result + (i * sizeof(struct sockaddr_in6)));
+            addr_v6 = (struct sockaddr_in6 *) &addr;
             r = inet_ntop(AF_INET6, (const void *) &addr_v6->sin6_addr, tmp, sizeof(tmp));
         }
         if (r) {
             retval.push_back(tmp);
         }
     }
 }
-}  // namespace network
 }  // namespace swoole
diff --git a/src/os/base.cc b/src/os/base.cc
@@ -85,14 +85,14 @@ namespace async {
 
 void handler_gethostbyname(AsyncEvent *event) {
     char addr[INET6_ADDRSTRLEN];
-    auto request = (GethostbynameRequest *) event->data;
-    int ret = network::gethostbyname(request->family, request->name, addr);
-    sw_memset_zero(request->addr, request->addr_len);
+    auto req = dynamic_cast<GethostbynameRequest *>(event->data.get());
+    int ret = network::gethostbyname(req->family, req->name.c_str(), addr);
+    sw_memset_zero(req->addr, req->addr_len);
 
     if (ret < 0) {
         event->error = SW_ERROR_DNSLOOKUP_RESOLVE_FAILED;
     } else {
-        if (inet_ntop(request->family, addr, request->addr, request->addr_len) == nullptr) {
+        if (inet_ntop(req->family, addr, req->addr, req->addr_len) == nullptr) {
             ret = -1;
             event->error = SW_ERROR_BAD_IPV6_ADDRESS;
         } else {
@@ -104,7 +104,7 @@ void handler_gethostbyname(AsyncEvent *event) {
 }
 
 void handler_getaddrinfo(AsyncEvent *event) {
-    network::GetaddrinfoRequest *req = (network::GetaddrinfoRequest *) event->data;
+    auto req = dynamic_cast<GetaddrinfoRequest *>(event->data.get());
     event->retval = network::getaddrinfo(req);
     event->error = req->error;
 }

diff --git a/tests/include/functions.php b/tests/include/functions.php
@@ -852,3 +852,11 @@ function build_ftp_url(string $path = ''): string
 {
     return 'ftp://' . FTP_USER . ':' . FTP_PASS . '@' . FTP_HOST . ':' .  FTP_PORT . '/' . $path;
 }
+
+function mkdir_if_not_exists(string $string): void
+{
+    if (!is_dir($string)) {
+        mkdir($string, 0777, true);
+    }
+}
+
diff --git a/tests/include/skipif.inc b/tests/include/skipif.inc
@@ -45,7 +45,7 @@ function swoole_color(string $content, int $color): string
 function skip(string $reason, bool $is_skip = true, int $color = SWOOLE_COLOR_YELLOW)
 {
     if ($is_skip) {
-        exit('skip ' . swoole_color($reason, $color));
+        exit('skip ' . swoole_color($reason, $color) . "\n");
     }
 }
 

diff --git a/tests/swoole_coroutine_system/getaddrinfo.phpt b/tests/swoole_coroutine_system/getaddrinfo.phpt
@@ -7,7 +7,7 @@ skip_if_offline();
 --FILE--
 <?php
 require __DIR__ . '/../include/bootstrap.php';
-go(function () {
+Co\run(function () {
     $ip_list = Swoole\Coroutine\System::getaddrinfo('www.baidu.com', AF_INET);
     Assert::assert(!empty($ip_list) and is_array($ip_list));
     foreach ($ip_list as $ip) {

diff --git a/tests/swoole_coroutine_system/getaddrinfo_timeout.phpt b/tests/swoole_coroutine_system/getaddrinfo_timeout.phpt
@@ -0,0 +1,30 @@
+--TEST--
+swoole_coroutine_system: getaddrinfo timeout
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc';
+skip_if_not_root();
+skip_if_in_ci();
+?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+$tmpdir = '/tmp/' . uniqid();
+mkdir_if_not_exists($tmpdir . '/etc');
+chroot($tmpdir);
+if (!is_file('/etc/resolv.conf')) {
+    file_put_contents('/etc/resolv.conf', "nameserver 192.168.8.8\noptions timeout:1 retry:1\n");
+}
+Co\run(static function () {
+    $res = Swoole\Coroutine\System::getaddrinfo(
+        domain: 'swoole-non-existent-domain',
+        protocol: 0,
+        service: '',
+        timeout: 0.5,
+    );
+    Assert::false($res);
+    Assert::eq(swoole_last_error(), SWOOLE_ERROR_CO_TIMEDOUT);
+    echo "DONE\n";
+});
+?>
+--EXPECT--
+DONE
diff --git a/tests/swoole_http_server/bug_5186.phpt b/tests/swoole_http_server/bug_5186.phpt
@@ -1,5 +1,5 @@
 --TEST--
-swoole_http_server: 在使用hyperf3的时候遇到Context::parse_multipart_data()的提示
+swoole_http_server: GitHub issue #5186
 --SKIPIF--
 <?php require __DIR__ . '/../include/skipif.inc'; ?>
 --FILE--