From 9105da7d0c87c3bbb1661c9baad5e7f00ee9b896 Mon Sep 17 00:00:00 2001 From: NathanFreeman <1056159381@qq.com> Date: Sun, 24 Nov 2024 14:41:50 +0800 Subject: [PATCH] Fix bug #5547 --- core-tests/src/network/dns.cpp | 16 +-------- include/swoole_async.h | 36 ++++++++++++++++--- include/swoole_socket.h | 15 ++------ src/coroutine/system.cc | 24 ++++--------- src/network/client.cc | 14 +++----- src/network/dns.cc | 29 ++++++++------- src/os/base.cc | 10 +++--- tests/include/functions.php | 8 +++++ tests/include/skipif.inc | 2 +- .../swoole_coroutine_system/getaddrinfo.phpt | 2 +- .../getaddrinfo_timeout.phpt | 30 ++++++++++++++++ tests/swoole_http_server/bug_5186.phpt | 2 +- 12 files changed, 107 insertions(+), 81 deletions(-) create mode 100644 tests/swoole_coroutine_system/getaddrinfo_timeout.phpt diff --git a/core-tests/src/network/dns.cpp b/core-tests/src/network/dns.cpp index cc7cd230024..76c6a4efc2f 100644 --- 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 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 index f5c718d2c0d..3ab5f5c745c 100644 --- 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 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 results; + int count; + void parse_result(std::vector &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 index 1626d8d09d0..c1990adea0b 100644 --- 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 &retval); -}; +namespace network { struct SendfileTask { off_t offset; diff --git a/src/coroutine/system.cc b/src/coroutine/system.cc index 001d66592f3..087e8712ced 100644 --- 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(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 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(req); coroutine::async(async::handler_getaddrinfo, ev, timeout); std::vector 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 index 816265c9a7c..6706defa2ce 100644 --- 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(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(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 index b96172ca017..22ac3ab9dd8 100644 --- a/src/network/dns.cc +++ b/src/network/dns.cc @@ -344,7 +344,7 @@ std::vector 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 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 &retval) { struct sockaddr_in *addr_v4; @@ -805,12 +809,12 @@ void GetaddrinfoRequest::parse_result(std::vector &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) { @@ -818,5 +822,4 @@ void GetaddrinfoRequest::parse_result(std::vector &retval) { } } } -} // namespace network } // namespace swoole diff --git a/src/os/base.cc b/src/os/base.cc index 9d6c2d10576..5b30d56d141 100644 --- 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(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(event->data.get()); event->retval = network::getaddrinfo(req); event->error = req->error; } diff --git a/tests/include/functions.php b/tests/include/functions.php index 4dd94504cad..d86b5e97ff5 100644 --- 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 index faf324acd13..1f731049d36 100644 --- 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 index 5fda59a59b8..87e91851607 100644 --- a/tests/swoole_coroutine_system/getaddrinfo.phpt +++ b/tests/swoole_coroutine_system/getaddrinfo.phpt @@ -7,7 +7,7 @@ skip_if_offline(); --FILE-- +--FILE-- + +--EXPECT-- +DONE diff --git a/tests/swoole_http_server/bug_5186.phpt b/tests/swoole_http_server/bug_5186.phpt index c6a89a874f2..b6672c216ea 100644 --- 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-- --FILE--