From d7e592d797173aac4cf36f9cd87f32c1f6f6147b Mon Sep 17 00:00:00 2001 From: hniksic Date: Fri, 7 Nov 2003 18:57:51 -0800 Subject: [PATCH] [svn] Replace conaddr with socket_ip_address. --- src/ChangeLog | 22 ++++++ src/connect.c | 20 ++++-- src/connect.h | 8 ++- src/ftp-basic.c | 21 ++---- src/ftp.c | 65 ++++++++--------- src/host.c | 75 ++++++++------------ src/host.h | 3 +- src/http.c | 184 +++++++++++++++++++++++++++--------------------- 8 files changed, 210 insertions(+), 188 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 85b6c5cb..c305d991 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,25 @@ +2003-11-08 Hrvoje Niksic + + * http.c (persistent_available_p): Instead of matching all the + addresses of HOST and last host, determine the peer's IP address + with socket_ip_address and see if that address is one of those + HOST resolves to. + + * host.c (address_list_match_all): Removed. + (address_list_find): New function, finds an IP address in the + address list. + + * ftp.c (ftp_do_pasv): Get the peer's address here, and pass it to + ftp_epsv so it doesn't need to call getpeername. + + * ftp-basic.c (ftp_port): Use socket_ip_address instead of + getpeername. + (ftp_lprt): Ditto. + + * connect.c (socket_ip_address): Replaces conaddr, generalized to + either get peer's or local address. + (sockaddr_get_data): Made local to this file. + 2003-11-08 Hrvoje Niksic * hash.c (HASH_POSITION): Explicitly accept the hash function. diff --git a/src/connect.c b/src/connect.c index a88e2595..25c15962 100644 --- a/src/connect.c +++ b/src/connect.c @@ -106,7 +106,7 @@ sockaddr_set_data (struct sockaddr *sa, const ip_address *ip, int port) you're not interested in one or the other information, pass NULL as the pointer. */ -void +static void sockaddr_get_data (const struct sockaddr *sa, ip_address *ip, int *port) { switch (sa->sa_family) @@ -510,16 +510,28 @@ acceptport (int local_sock, int *sock) return ACCEPTOK; } -/* Return the local IP address associated with the connection on FD. */ +/* Get the IP address associated with the connection on FD and store + it to IP. Return 1 on success, 0 otherwise. + + If ENDPOINT is ENDPOINT_LOCAL, it returns the address of the local + (client) side of the socket. Else if ENDPOINT is ENDPOINT_PEER, it + returns the address of the remote (peer's) side of the socket. */ int -conaddr (int fd, ip_address *ip) +socket_ip_address (int sock, ip_address *ip, int endpoint) { struct sockaddr_storage storage; struct sockaddr *sockaddr = (struct sockaddr *)&storage; socklen_t addrlen = sizeof (storage); + int ret; - if (getsockname (fd, sockaddr, &addrlen) < 0) + if (endpoint == ENDPOINT_LOCAL) + ret = getsockname (sock, sockaddr, &addrlen); + else if (endpoint == ENDPOINT_PEER) + ret = getpeername (sock, sockaddr, &addrlen); + else + abort (); + if (ret < 0) return 0; switch (sockaddr->sa_family) diff --git a/src/connect.h b/src/connect.h index 271f32ed..aa485c1c 100644 --- a/src/connect.h +++ b/src/connect.h @@ -51,10 +51,14 @@ enum { int connect_to_host PARAMS ((const char *, int)); int connect_to_ip PARAMS ((const ip_address *, int, const char *)); -void sockaddr_get_data PARAMS ((const struct sockaddr *, ip_address *, int *)); uerr_t bindport PARAMS ((const ip_address *, int *, int *)); uerr_t acceptport PARAMS ((int, int *)); -int conaddr PARAMS ((int, ip_address *)); + +enum { + ENDPOINT_LOCAL, + ENDPOINT_PEER +}; +int socket_ip_address PARAMS ((int, ip_address *, int)); /* Flags for select_fd's WAIT_FOR argument. */ enum { diff --git a/src/ftp-basic.c b/src/ftp-basic.c index 469263be..5c60ccfb 100644 --- a/src/ftp-basic.c +++ b/src/ftp-basic.c @@ -285,7 +285,7 @@ ftp_port (struct rbuf *rbuf, int *local_sock) assert (rbuf_initialized_p (rbuf)); /* Get the address of this side of the connection. */ - if (!conaddr (RBUF_FD (rbuf), &addr)) + if (!socket_ip_address (RBUF_FD (rbuf), &addr, ENDPOINT_LOCAL)) return BINDERR; assert (addr.type == IPV4_ADDRESS); @@ -382,7 +382,7 @@ ftp_lprt (struct rbuf *rbuf, int *local_sock) assert (rbuf_initialized_p (rbuf)); /* Get the address of this side of the connection. */ - if (!conaddr (RBUF_FD (rbuf), &addr)) + if (!socket_ip_address (RBUF_FD (rbuf), &addr, ENDPOINT_LOCAL)) return BINDERR; assert (addr.type == IPV4_ADDRESS || addr.type == IPV6_ADDRESS); @@ -466,7 +466,7 @@ ftp_eprt (struct rbuf *rbuf, int *local_sock) assert (rbuf_initialized_p(rbuf)); /* Get the address of this side of the connection. */ - if (!conaddr (RBUF_FD (rbuf), &addr)) + if (!socket_ip_address (RBUF_FD (rbuf), &addr, ENDPOINT_LOCAL)) return BINDERR; assert (addr.type == IPV4_ADDRESS || addr.type == IPV6_ADDRESS); @@ -756,27 +756,18 @@ ftp_epsv (struct rbuf *rbuf, ip_address *ip, int *port) int nwritten, i; uerr_t err; int tport; - socklen_t addrlen; - struct sockaddr_storage ss; - struct sockaddr *sa = (struct sockaddr *)&ss; assert (rbuf != NULL); assert (rbuf_initialized_p(rbuf)); assert (ip != NULL); assert (port != NULL); - addrlen = sizeof (ss); - if (getpeername (rbuf->fd, sa, &addrlen) < 0) - /* Mauro Tortonesi: HOW DO WE HANDLE THIS ERROR? */ - return CONPORTERR; - - assert (sa->sa_family == AF_INET || sa->sa_family == AF_INET6); - - sockaddr_get_data (sa, ip, NULL); + /* IP already contains the IP address of the control connection's + peer, so we don't need to call socket_ip_address here. */ /* Form the request. */ /* EPSV 1 means that we ask for IPv4 and EPSV 2 means that we ask for IPv6. */ - request = ftp_request ("EPSV", (sa->sa_family == AF_INET ? "1" : "2")); + request = ftp_request ("EPSV", (ip->type == IPV4_ADDRESS ? "1" : "2")); /* And send it. */ nwritten = xwrite (RBUF_FD (rbuf), request, strlen (request), -1); diff --git a/src/ftp.c b/src/ftp.c index 7e878e42..adc74a1e 100644 --- a/src/ftp.c +++ b/src/ftp.c @@ -123,22 +123,6 @@ ftp_expected_bytes (const char *s) } #ifdef ENABLE_IPV6 -static int -getfamily (int fd) -{ - struct sockaddr_storage ss; - struct sockaddr *sa = (struct sockaddr *)&ss; - socklen_t len = sizeof (ss); - - assert (fd >= 0); - - if (getpeername (fd, sa, &len) < 0) - /* Mauro Tortonesi: HOW DO WE HANDLE THIS ERROR? */ - abort (); - - return sa->sa_family; -} - /* * This function sets up a passive data connection with the FTP server. * It is merely a wrapper around ftp_epsv, ftp_lpsv and ftp_pasv. @@ -147,16 +131,24 @@ static uerr_t ftp_do_pasv (struct rbuf *rbuf, ip_address *addr, int *port) { uerr_t err; - int family; - family = getfamily (rbuf->fd); - assert (family == AF_INET || family == AF_INET6); + /* We need to determine the address family and need to call + getpeername, so while we're at it, store the address to ADDR. + ftp_pasv and ftp_lpsv can simply override it. */ + if (!socket_ip_address (RBUF_FD (rbuf), addr, ENDPOINT_PEER)) + abort (); /* If our control connection is over IPv6, then we first try EPSV and then * LPSV if the former is not supported. If the control connection is over * IPv4, we simply issue the good old PASV request. */ - if (family == AF_INET6) + switch (addr->type) { + case IPV4_ADDRESS: + if (!opt.server_response) + logputs (LOG_VERBOSE, "==> PASV ... "); + err = ftp_pasv (rbuf, addr, port); + break; + case IPV6_ADDRESS: if (!opt.server_response) logputs (LOG_VERBOSE, "==> EPSV ... "); err = ftp_epsv (rbuf, addr, port); @@ -168,12 +160,9 @@ ftp_do_pasv (struct rbuf *rbuf, ip_address *addr, int *port) logputs (LOG_VERBOSE, "==> LPSV ... "); err = ftp_lpsv (rbuf, addr, port); } - } - else - { - if (!opt.server_response) - logputs (LOG_VERBOSE, "==> PASV ... "); - err = ftp_pasv (rbuf, addr, port); + break; + default: + abort (); } return err; @@ -187,19 +176,25 @@ static uerr_t ftp_do_port (struct rbuf *rbuf, int *local_sock) { uerr_t err; - int family; + ip_address cip; assert (rbuf != NULL); assert (rbuf_initialized_p (rbuf)); - family = getfamily (rbuf->fd); - assert (family == AF_INET || family == AF_INET6); + if (!socket_ip_address (RBUF_FD (rbuf), &cip, ENDPOINT_PEER)) + abort (); /* If our control connection is over IPv6, then we first try EPRT and then * LPRT if the former is not supported. If the control connection is over * IPv4, we simply issue the good old PORT request. */ - if (family == AF_INET6) + switch (cip.type) { + case IPV4_ADDRESS: + if (!opt.server_response) + logputs (LOG_VERBOSE, "==> PORT ... "); + err = ftp_port (rbuf, local_sock); + break; + case IPV6_ADDRESS: if (!opt.server_response) logputs (LOG_VERBOSE, "==> EPRT ... "); err = ftp_eprt (rbuf, local_sock); @@ -211,14 +206,10 @@ ftp_do_port (struct rbuf *rbuf, int *local_sock) logputs (LOG_VERBOSE, "==> LPRT ... "); err = ftp_lprt (rbuf, local_sock); } + break; + default: + abort (); } - else - { - if (!opt.server_response) - logputs (LOG_VERBOSE, "==> PORT ... "); - err = ftp_port (rbuf, local_sock); - } - return err; } #else diff --git a/src/host.c b/src/host.c index 81c8a54a..81483572 100644 --- a/src/host.c +++ b/src/host.c @@ -124,61 +124,44 @@ address_list_address_at (const struct address_list *al, int pos) return al->addresses + pos; } -/* Check whether two address lists have all their IPs in common. */ +/* Return 1 if IP is one of the addresses in AL. */ int -address_list_match_all (const struct address_list *al1, - const struct address_list *al2) +address_list_find (const struct address_list *al, const ip_address *ip) { -#ifdef ENABLE_IPV6 int i; -#endif - if (al1 == al2) - return 1; - if (al1->count != al2->count) - return 0; - - /* For the comparison to be complete, we'd need to sort the IP - addresses first. But that's not necessary because this is only - used as an optimization. */ - -#ifndef ENABLE_IPV6 - /* In the non-IPv6 case, there is only one address type, so we can - compare the whole array with memcmp. */ - return 0 == memcmp (al1->addresses, al2->addresses, - al1->count * sizeof (ip_address)); -#else /* ENABLE_IPV6 */ - for (i = 0; i < al1->count; ++i) + switch (ip->type) { - const ip_address *ip1 = &al1->addresses[i]; - const ip_address *ip2 = &al2->addresses[i]; - - if (ip1->type != ip2->type) - return 0; - - switch (ip1->type) + case IPV4_ADDRESS: + for (i = 0; i < al->count; i++) { - case IPV4_ADDRESS: - if (ADDRESS_IPV4_IN_ADDR (ip1).s_addr - != - ADDRESS_IPV4_IN_ADDR (ip2).s_addr) - return 0; - break; - case IPV6_ADDRESS: + ip_address *cur = al->addresses + i; + if (cur->type == IPV4_ADDRESS + && (ADDRESS_IPV4_IN_ADDR (cur).s_addr + == + ADDRESS_IPV4_IN_ADDR (ip).s_addr)) + return 1; + } + return 0; +#ifdef ENABLE_IPV6 + case IPV6_ADDRESS: + for (i = 0; i < al->count; i++) + { + ip_address *cur = al->addresses + i; + if (cur->type == IPV6_ADDRESS #ifdef HAVE_SOCKADDR_IN6_SCOPE_ID - if (ADDRESS_IPV6_SCOPE (ip1) != ADDRESS_IPV6_SCOPE (ip2)) - return 0; -#endif /* HAVE_SOCKADDR_IN6_SCOPE_ID */ - if (!IN6_ARE_ADDR_EQUAL (&ADDRESS_IPV6_IN6_ADDR (ip1), - &ADDRESS_IPV6_IN6_ADDR (ip2))) - return 0; - break; - default: - abort (); + && ADDRESS_IPV6_SCOPE (cur) == ADDRESS_IPV6_SCOPE (ip) +#endif + && IN6_ARE_ADDR_EQUAL (&ADDRESS_IPV6_IN6_ADDR (cur), + &ADDRESS_IPV6_IN6_ADDR (ip))) + return 1; } - } - return 1; + return 0; #endif /* ENABLE_IPV6 */ + default: + abort (); + return 1; + } } /* Mark the INDEXth element of AL as faulty, so that the next time diff --git a/src/host.h b/src/host.h index 24e98014..1bb514ca 100644 --- a/src/host.h +++ b/src/host.h @@ -108,8 +108,7 @@ void address_list_get_bounds PARAMS ((const struct address_list *, int address_list_cached_p PARAMS ((const struct address_list *)); const ip_address *address_list_address_at PARAMS ((const struct address_list *, int)); -int address_list_match_all PARAMS ((const struct address_list *, - const struct address_list *)); +int address_list_find PARAMS ((const struct address_list *, const ip_address *)); void address_list_set_faulty PARAMS ((struct address_list *, int)); void address_list_release PARAMS ((struct address_list *)); diff --git a/src/http.c b/src/http.c index 031bff75..7be33a63 100644 --- a/src/http.c +++ b/src/http.c @@ -343,40 +343,36 @@ http_process_set_cookie (const char *hdr, void *arg) /* Persistent connections. Currently, we cache the most recently used connection as persistent, provided that the HTTP server agrees to make it such. The persistence data is stored in the variables - below. Ideally, it would be in a structure, and it should be - possible to cache an arbitrary fixed number of these connections. - - I think the code is quite easy to extend in that direction. */ + below. Ideally, it should be possible to cache an arbitrary fixed + number of these connections. */ /* Whether a persistent connection is active. */ -static int pc_active_p; +static int pconn_active; -/* Host and port of currently active persistent connection. */ -static struct address_list *pc_last_host_ip; -static unsigned short pc_last_port; +static struct { + /* The socket of the connection. */ + int socket; -/* File descriptor of the currently active persistent connection. */ -static int pc_last_fd; + /* Host and port of the currently active persistent connection. */ + char *host; + int port; -/* Whether a ssl handshake has occoured on this connection */ -static int pc_last_ssl_p; + /* Whether a ssl handshake has occoured on this connection. */ + int ssl; +} pconn; -/* Mark the persistent connection as invalid. This is used by the - CLOSE_* macros after they forcefully close a registered persistent - connection. This does not close the file descriptor -- it is left - to the caller to do that. (Maybe it should, though.) */ +/* Mark the persistent connection as invalid and free the resources it + uses. This is used by the CLOSE_* macros after they forcefully + close a registered persistent connection. */ static void invalidate_persistent (void) { - pc_active_p = 0; - pc_last_ssl_p = 0; - if (pc_last_host_ip != NULL) - { - address_list_release (pc_last_host_ip); - pc_last_host_ip = NULL; - } - DEBUGP (("Invalidating fd %d from further reuse.\n", pc_last_fd)); + DEBUGP (("Disabling further reuse of socket %d.\n", pconn.socket)); + pconn_active = 0; + xclose (pconn.socket); + xfree (pconn.host); + xzero (pconn); } /* Register FD, which should be a TCP/IP connection to HOST:PORT, as @@ -388,94 +384,108 @@ invalidate_persistent (void) If a previous connection was persistent, it is closed. */ static void -register_persistent (const char *host, unsigned short port, int fd, int ssl) +register_persistent (const char *host, int port, int fd, int ssl) { - if (pc_active_p) + if (pconn_active) { - if (pc_last_fd == fd) + if (pconn.socket == fd) { - /* The connection FD is already registered. Nothing to - do. */ + /* The connection FD is already registered. */ return; } else { - /* The old persistent connection is still active; let's - close it first. This situation arises whenever a - persistent connection exists, but we then connect to a - different host, and try to register a persistent - connection to that one. */ - xclose (pc_last_fd); + /* The old persistent connection is still active; close it + first. This situation arises whenever a persistent + connection exists, but we then connect to a different + host, and try to register a persistent connection to that + one. */ invalidate_persistent (); } } - assert (pc_last_host_ip == NULL); + pconn_active = 1; + pconn.socket = fd; + pconn.host = xstrdup (host); + pconn.port = port; + pconn.ssl = ssl; - /* This lookup_host cannot fail, because it has the results in the - cache. */ - pc_last_host_ip = lookup_host (host, LH_SILENT); - assert (pc_last_host_ip != NULL); - - pc_last_port = port; - pc_last_fd = fd; - pc_active_p = 1; - pc_last_ssl_p = ssl; - DEBUGP (("Registered fd %d for persistent reuse.\n", fd)); + DEBUGP (("Registered socket %d for persistent reuse.\n", fd)); } /* Return non-zero if a persistent connection is available for connecting to HOST:PORT. */ static int -persistent_available_p (const char *host, unsigned short port, int ssl) +persistent_available_p (const char *host, int port, int ssl, + int *host_lookup_failed) { - int success; - struct address_list *this_host_ip; - /* First, check whether a persistent connection is active at all. */ - if (!pc_active_p) - return 0; - /* Second, check if the active connection pertains to the correct - (HOST, PORT) ordered pair. */ - if (port != pc_last_port) + if (!pconn_active) return 0; - /* Second, a): check if current connection is (not) ssl, too. This - test is unlikely to fail because HTTP and HTTPS typicaly use - different ports. Yet it is possible, or so I [Christian - Fraenkel] have been told, to run HTTPS and HTTP simultaneus on - the same port. */ - if (ssl != pc_last_ssl_p) + /* If we want SSL and the last connection wasn't or vice versa, + don't use it. Checking for host and port is not enough because + HTTP and HTTPS can apparently coexist on the same port. */ + if (ssl != pconn.ssl) return 0; - this_host_ip = lookup_host (host, 0); - if (!this_host_ip) + /* If we're not connecting to the same port, we're not interested. */ + if (port != pconn.port) return 0; - /* To equate the two host names for the purposes of persistent - connections, they need to share all the IP addresses in the - list. */ - success = address_list_match_all (pc_last_host_ip, this_host_ip); - address_list_release (this_host_ip); - if (!success) - return 0; + /* If the host is the same, we're in business. If not, there is + still hope -- read below. */ + if (0 != strcasecmp (host, pconn.host)) + { + /* This is somewhat evil, but works in practice: if the address + that pconn.socket is connected to is one of the IP addresses + HOST resolves to, we don't need to reconnect. #### Is it + correct to do this by default? */ + int found; + ip_address ip; + struct address_list *al; + + if (!socket_ip_address (pconn.socket, &ip, 0)) + { + /* Can't get the peer's address -- something must be wrong + with the connection. */ + invalidate_persistent (); + return 0; + } + al = lookup_host (host, 0); + if (!al) + { + *host_lookup_failed = 1; + return 0; + } + + found = address_list_find (al, &ip); + address_list_release (al); + + if (!found) + return 0; + + /* HOST resolves to an address pconn.sock is connected to -- no + need to reconnect. */ + } - /* Third: check whether the connection is still open. This is + /* Finally, check whether the connection is still open. This is important because most server implement a liberal (short) timeout on persistent connections. Wget can of course always reconnect if the connection doesn't work out, but it's nicer to know in advance. This test is a logical followup of the first test, but is "expensive" and therefore placed at the end of the list. */ - if (!test_socket_open (pc_last_fd)) + + if (!test_socket_open (pconn.socket)) { /* Oops, the socket is no longer open. Now that we know that, let's invalidate the persistent connection before returning 0. */ - xclose (pc_last_fd); invalidate_persistent (); return 0; } + return 1; } @@ -497,16 +507,18 @@ persistent_available_p (const char *host, unsigned short port, int ssl) #define CLOSE_FINISH(fd) do { \ if (!keep_alive) \ { \ - xclose (fd); \ - if (pc_active_p && (fd) == pc_last_fd) \ + if (pconn_active && (fd) == pconn.socket) \ invalidate_persistent (); \ + else \ + xclose (fd); \ } \ } while (0) #define CLOSE_INVALIDATE(fd) do { \ - xclose (fd); \ - if (pc_active_p && (fd) == pc_last_fd) \ + if (pconn_active && (fd) == pconn.socket) \ invalidate_persistent (); \ + else \ + xclose (fd); \ } while (0) struct http_stat @@ -606,6 +618,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy) char *post_content_type, *post_content_length; long post_data_size = 0; + int host_lookup_failed; + #ifdef HAVE_SSL /* Initialize the SSL context. After the first run, this is a no-op. */ @@ -668,6 +682,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy) server. */ conn = proxy ? proxy : u; + host_lookup_failed = 0; + /* First: establish the connection. */ if (inhibit_keep_alive || !persistent_available_p (conn->host, conn->port, @@ -676,8 +692,14 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy) #else 0 #endif - )) + , &host_lookup_failed)) { + /* In its current implementation, persistent_available_p will + look up conn->host in some cases. If that lookup failed, we + don't need to bother with connect_to_host. */ + if (host_lookup_failed) + return HOSTERR; + sock = connect_to_host (conn->host, conn->port); if (sock == E_HOST) return HOSTERR; @@ -705,8 +727,8 @@ gethttp (struct url *u, struct http_stat *hs, int *dt, struct url *proxy) conn->host, conn->port); /* #### pc_last_fd should be accessed through an accessor function. */ - sock = pc_last_fd; - using_ssl = pc_last_ssl_p; + sock = pconn.socket; + using_ssl = pconn.ssl; DEBUGP (("Reusing fd %d.\n", sock)); } @@ -2427,6 +2449,4 @@ create_authorization_line (const char *au, const char *user, void http_cleanup (void) { - if (pc_last_host_ip) - address_list_release (pc_last_host_ip); } -- 2.39.2