ghost
commented
5 years ago You are assuming I want strings to begin with. This is by design.
Closed BotoX closed 5 years ago
ghost
commented
5 years ago You are assuming I want strings to begin with. This is by design.
uniqss
commented
4 years ago static inline void internal_finalize_bsd_addr(struct bsd_addr_t *addr) { // parse, so to speak, the address if (addr->mem.ss_family == AF_INET6) { addr->ip = (char *) &((struct sockaddr_in6 *) addr)->sin6_addr; addr->ip_length = sizeof(struct in6_addr); } else if (addr->mem.ss_family == AF_INET) { addr->ip = (char *) &((struct sockaddr_in *) addr)->sin_addr; addr->ip_length = sizeof(struct in_addr); } else { addr->ip_length = 0; } }You are assuming here that sin6_addr and sin_addr from sockaddr_in6 and sockaddr_in are strings (char ). They are however just raw bytes: `struct in_addr sin_addr; / Internet address. */
and would need to be converted to strings withinet_ntop`. Example here: https://beej.us/guide/bgnet/html/multi/getpeernameman.htmlI'd suggest changing the whole thing like so:
* Remove ip and ip_length from struct bsd_addr_t. * Remove internal_finalize_bsd_addr. * Add bsd_addr_to_ipstr which does the addr to ip string conversion.Like so:
diff --git a/src/interfaces/socket.h b/src/interfaces/socket.h index e58395b..972e1bb 100644 --- a/src/interfaces/socket.h +++ b/src/interfaces/socket.h @@ -46,4 +46,4 @@ WIN32_EXPORT int us_socket_is_closed(struct us_socket *s); WIN32_EXPORT struct us_socket *us_socket_close(struct us_socket *s); /* Copy remote (IP) address of socket, or fail with zero length. */ -WIN32_EXPORT void us_socket_remote_address(struct us_socket *s, char *buf, int *length); \ No newline at end of file +WIN32_EXPORT int us_socket_remote_address(struct us_socket *s, char *buf, int length); \ No newline at end of file diff --git a/src/internal/common.h b/src/internal/common.h index 3de04c8..92d1aa6 100644 --- a/src/internal/common.h +++ b/src/internal/common.h @@ -74,7 +74,7 @@ struct us_socket_context { struct us_socket *iterator; struct us_socket_context *prev, *next; - struct us_socket *(*on_open)(struct us_socket *, int is_client, char *ip, int ip_length); + struct us_socket *(*on_open)(struct us_socket *, int is_client, char *ip, int port); struct us_socket *(*on_data)(struct us_socket *, char *data, int length); struct us_socket *(*on_writable)(struct us_socket *); struct us_socket *(*on_close)(struct us_socket *); diff --git a/src/internal/networking/bsd.h b/src/internal/networking/bsd.h index 57166b4..cd0d31d 100644 --- a/src/internal/networking/bsd.h +++ b/src/internal/networking/bsd.h @@ -34,6 +34,7 @@ #define _GNU_SOURCE #include <sys/types.h> #include <sys/socket.h> +#include <arpa/inet.h> #include <netinet/in.h> #include <netinet/tcp.h> #include <netdb.h> @@ -99,39 +100,30 @@ static inline void bsd_shutdown_socket(LIBUS_SOCKET_DESCRIPTOR fd) { struct bsd_addr_t { struct sockaddr_storage mem; socklen_t len; - char *ip; - int ip_length; }; -static inline void internal_finalize_bsd_addr(struct bsd_addr_t *addr) { - // parse, so to speak, the address - if (addr->mem.ss_family == AF_INET6) { - addr->ip = (char *) &((struct sockaddr_in6 *) addr)->sin6_addr; - addr->ip_length = sizeof(struct in6_addr); - } else if (addr->mem.ss_family == AF_INET) { - addr->ip = (char *) &((struct sockaddr_in *) addr)->sin_addr; - addr->ip_length = sizeof(struct in_addr); - } else { - addr->ip_length = 0; - } -} - static inline int bsd_socket_addr(LIBUS_SOCKET_DESCRIPTOR fd, struct bsd_addr_t *addr) { addr->len = sizeof(addr->mem); if (getpeername(fd, (struct sockaddr *) &addr->mem, &addr->len)) { return -1; } - internal_finalize_bsd_addr(addr); return 0; } -static inline char *bsd_addr_get_ip(struct bsd_addr_t *addr) { - return addr->ip; +static inline int bsd_addr_to_ipstr(struct bsd_addr_t *addr, char *str, int str_length) { + // parse, so to speak, the address + if (addr->mem.ss_family == AF_INET6) { + struct sockaddr_in6 *s = (struct sockaddr_in6 *)addr; + inet_ntop(AF_INET6, &s->sin6_addr, str, str_length); + return ntohs(s->sin6_port); + } else if (addr->mem.ss_family == AF_INET) { + struct sockaddr_in *s = (struct sockaddr_in *)addr; + inet_ntop(AF_INET, &s->sin_addr, str, str_length); + return ntohs(s->sin_port); + } + return -1; } -static inline int bsd_addr_get_ip_length(struct bsd_addr_t *addr) { - return addr->ip_length; -} // called by dispatch_ready_poll static inline LIBUS_SOCKET_DESCRIPTOR bsd_accept_socket(LIBUS_SOCKET_DESCRIPTOR fd, struct bsd_addr_t *addr) { @@ -146,8 +138,6 @@ static inline LIBUS_SOCKET_DESCRIPTOR bsd_accept_socket(LIBUS_SOCKET_DESCRIPTOR accepted_fd = accept(fd, (struct sockaddr *) addr, &addr->len); #endif - internal_finalize_bsd_addr(addr); - return apple_no_sigpipe(accepted_fd); } diff --git a/src/libusockets_new.h b/src/libusockets_new.h index 2259f49..f873f24 100644 --- a/src/libusockets_new.h +++ b/src/libusockets_new.h @@ -255,7 +255,7 @@ inline static int us_new_socket_is_shut_down(const int ssl, struct us_new_socket #endif } -inline static void us_new_socket_remote_address(const int ssl, struct us_new_socket_t *s, char *buf, int *length) { +inline static void us_new_socket_remote_address(const int ssl, struct us_new_socket_t *s, char *buf, int length) { /* There is no SSL variant of this function */ us_socket_remote_address((struct us_socket *) s, buf, length); } diff --git a/src/loop.c b/src/loop.c index 4f1ed32..aa316ff 100644 --- a/src/loop.c +++ b/src/loop.c @@ -187,7 +187,9 @@ void us_internal_dispatch_ready_poll(struct us_poll *p, int error, int events) { // make sure to link this socket into its context! us_internal_socket_context_link(listen_socket->s.context, s); - listen_socket->s.context->on_open(s, 0, bsd_addr_get_ip(&addr), bsd_addr_get_ip_length(&addr)); + char addr_str[INET6_ADDRSTRLEN]; + int port = bsd_addr_to_ipstr(&addr, addr_str, sizeof(addr_str)); + listen_socket->s.context->on_open(s, 0, addr_str, port); } while ((client_fd = bsd_accept_socket(us_poll_fd(p), &addr)) != LIBUS_SOCKET_ERROR); } } diff --git a/src/socket.c b/src/socket.c index 1f0b65b..e208027 100644 --- a/src/socket.c +++ b/src/socket.c @@ -23,14 +23,11 @@ void us_internal_init_socket(struct us_socket *s) { // shared nodelay here? } -void us_socket_remote_address(struct us_socket *s, char *buf, int *length) { +int us_socket_remote_address(struct us_socket *s, char *buf, int length) { struct bsd_addr_t addr; - if (bsd_socket_addr(us_poll_fd(&s->p), &addr) || *length < bsd_addr_get_ip_length(&addr)) { - *length = 0; - } else { - *length = bsd_addr_get_ip_length(&addr); - memcpy(buf, bsd_addr_get_ip(&addr), *length); - } + if (bsd_socket_addr(us_poll_fd(&s->p), &addr)) + return -1; + return bsd_addr_to_ipstr(&addr, buf, length); } int us_socket_write(struct us_socket *s, const char *data, int length, int msg_more) {I'd also love to have a function that returns the bsd_addr_t / sockaddr instead of a string so I don't have to convert the string back to a sockaddr again if I need that. But it seems like you are trying to keep all the OS stuff separate so I guess can't return a sockaddr anywhere in the API.
Hello, @BotoX . I tried your code, it works, but when you use ip address 4, then it will print something like this:
I0101 16:13:18.676148 5662 tcp_server.cpp:89] on_tcp_socket_open ip:::ffff:111.22.111.222 port:28012
BotoX
commented
4 years ago @uniqss I've dropped the code I wrote in this issue for the following solution in my program:
std::string_view remote_ip = hres.getRemoteAddress();
if(remote_ip.length() == sizeof(in6_addr))
{
memcpy(&req.addr6.addr, remote_ip.data(), sizeof(req.addr6.addr));
if(req.addr6.addr.__in6_u.__u6_addr32[0] == 0x00 &&
req.addr6.addr.__in6_u.__u6_addr32[1] == 0x00 &&
req.addr6.addr.__in6_u.__u6_addr32[2] == 0xFFFF0000)
{
req.addr4.addr.s_addr = req.addr6.addr.__in6_u.__u6_addr32[3];
}
else
req.isIPv6 = true;
}
else
{
memcpy(&req.addr4.addr, remote_ip.data(), sizeof(req.addr4.addr));
}https://git.botox.bz/BotoX/misaka/src/branch/master/src/parser.cpp#L334
The issue you are having is normal behavior however when you are listening with IPv4 and IPv6 on the same socket.
You are assuming here that sin6_addr and sin_addr from sockaddr_in6 and sockaddr_in are strings (char ). They are however just raw bytes: `struct in_addr sin_addr; / Internet address. */
and would need to be converted to strings withinet_ntop`. Example here: https://beej.us/guide/bgnet/html/multi/getpeernameman.htmlI'd suggest changing the whole thing like so:
Like so:
I'd also love to have a function that returns the bsd_addr_t / sockaddr instead of a string so I don't have to convert the string back to a sockaddr again if I need that. But it seems like you are trying to keep all the OS stuff separate so I guess can't return a sockaddr anywhere in the API.