[6.0] 5852ed5cf Introduce a VSS_ResolveOne() function and use it the places where we want a single unique suckaddr.

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Wed May 22 09:03:11 UTC 2019


commit 5852ed5cf63d9e91d5242d5605a546fb19d00a3a
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon May 6 19:55:58 2019 +0000

    Introduce a VSS_ResolveOne() function and use it the places where
    we want a single unique suckaddr.
    
    Conflicts:
            bin/varnishtest/vtc_client.c
            lib/libvmod_std/vmod_std_conversions.c

diff --git a/bin/varnishd/proxy/cache_proxy_proto.c b/bin/varnishd/proxy/cache_proxy_proto.c
index fc4c451b3..1def50ebe 100644
--- a/bin/varnishd/proxy/cache_proxy_proto.c
+++ b/bin/varnishd/proxy/cache_proxy_proto.c
@@ -40,6 +40,7 @@
 
 #include "vend.h"
 #include "vsa.h"
+#include "vss.h"
 #include "vtcp.h"
 
 struct vpx_tlv {
@@ -61,7 +62,6 @@ vpx_proto1(const struct worker *wrk, const struct req *req)
 	const char *fld[5];
 	int i;
 	char *p, *q;
-	struct addrinfo hints, *res;
 	struct suckaddr *sa;
 	int pfam = -1;
 
@@ -100,58 +100,34 @@ vpx_proto1(const struct worker *wrk, const struct req *req)
 	}
 
 	if (!strcmp(fld[0], "TCP4"))
-		pfam = AF_INET;
+		pfam = PF_INET;
 	else if (!strcmp(fld[0], "TCP6"))
-		pfam = AF_INET6;
+		pfam = PF_INET6;
 	else {
 		VSL(SLT_ProxyGarbage, req->sp->vxid,
 		    "PROXY1: Wrong TCP[46] field");
 		return (-1);
 	}
 
-	memset(&hints, 0, sizeof hints);
-	hints.ai_socktype = SOCK_STREAM;
-	hints.ai_flags = AI_NUMERICHOST | AI_NUMERICSERV;
-
-	i = getaddrinfo(fld[1], fld[3], &hints, &res);
-	if (i != 0) {
-		VSL(SLT_ProxyGarbage, req->sp->vxid,
-		    "PROXY1: Cannot resolve source address (%s)",
-		    gai_strerror(i));
-		return (-1);
-	}
-	AZ(res->ai_next);
-	if (res->ai_family != pfam) {
+	SES_Reserve_client_addr(req->sp, &sa);
+	if (VSS_ResolveOne(sa, fld[1], fld[3],
+	    pfam, SOCK_STREAM, AI_NUMERICHOST | AI_NUMERICSERV) == NULL) {
 		VSL(SLT_ProxyGarbage, req->sp->vxid,
-		    "PROXY1: %s got wrong protocol (%d)",
-		    fld[0], res->ai_family);
-		freeaddrinfo(res);
+		    "PROXY1: Cannot resolve source address");
 		return (-1);
 	}
-	SES_Reserve_client_addr(req->sp, &sa);
-	AN(VSA_Build(sa, res->ai_addr, res->ai_addrlen));
 	SES_Set_String_Attr(req->sp, SA_CLIENT_IP, fld[1]);
 	SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, fld[3]);
-	freeaddrinfo(res);
 
-	i = getaddrinfo(fld[2], fld[4], &hints, &res);
-	if (i != 0) {
-		VSL(SLT_ProxyGarbage, req->sp->vxid,
-		    "PROXY1: Cannot resolve destination address (%s)",
-		    gai_strerror(i));
-		return (-1);
-	}
-	AZ(res->ai_next);
-	if (res->ai_family != pfam) {
+	SES_Reserve_server_addr(req->sp, &sa);
+	if (VSS_ResolveOne(sa, fld[2], fld[4],
+	    pfam, SOCK_STREAM, AI_NUMERICHOST | AI_NUMERICSERV) == NULL) {
 		VSL(SLT_ProxyGarbage, req->sp->vxid,
-		    "PROXY1: %s got wrong protocol (%d)",
-		    fld[0], res->ai_family);
-		freeaddrinfo(res);
+		    "PROXY1: Cannot resolve destination address");
 		return (-1);
 	}
-	SES_Reserve_server_addr(req->sp, &sa);
-	AN(VSA_Build(sa, res->ai_addr, res->ai_addrlen));
-	freeaddrinfo(res);
+	SES_Set_String_Attr(req->sp, SA_CLIENT_IP, fld[1]);
+	SES_Set_String_Attr(req->sp, SA_CLIENT_PORT, fld[3]);
 
 	VSL(SLT_Proxy, req->sp->vxid, "1 %s %s %s %s",
 	    fld[1], fld[3], fld[2], fld[4]);
diff --git a/bin/varnishtest/tests/o00000.vtc b/bin/varnishtest/tests/o00000.vtc
index 1e5dcabf8..08459fbe4 100644
--- a/bin/varnishtest/tests/o00000.vtc
+++ b/bin/varnishtest/tests/o00000.vtc
@@ -34,10 +34,10 @@ logexpect l1 -v v1 {
 	expect * 1002	ProxyGarbage	"PROXY1: Too few fields"
 	expect * 1003	ProxyGarbage	"PROXY1: Too many fields"
 	expect * 1004	ProxyGarbage	"PROXY1: Wrong TCP\\[46\\] field"
-	expect * 1005	ProxyGarbage	"PROXY1: Cannot resolve source address \\(.*\\)"
-	expect * 1007	ProxyGarbage	"PROXY1: Cannot resolve destination address \\(.*\\)"
-	expect * 1013	ProxyGarbage	"PROXY1: TCP4 got wrong protocol \\([0-9]*\\)"
-	expect * 1014	ProxyGarbage	"PROXY1: TCP6 got wrong protocol \\([0-9]*\\)"
+	expect * 1005	ProxyGarbage	"PROXY1: Cannot resolve source address"
+	expect * 1007	ProxyGarbage	"PROXY1: Cannot resolve destination address"
+	expect * 1009	ProxyGarbage	"PROXY1: Cannot resolve source address"
+	expect * 1011	ProxyGarbage	"PROXY1: Cannot resolve source address"
 	expect * 1015	Proxy		"1 1.2.3.4 1234 5.6.7.8 5678"
 	expect * 1018	Proxy		"1 1:f::2 1234 5:a::8 5678"
 	expect * 1021	Proxy		"1 1:f::3 1234 5:a::8 5678"
@@ -50,7 +50,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY "
@@ -58,7 +58,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY A B C D\r\n"
@@ -66,7 +66,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY A B C D E F\r\n"
@@ -74,7 +74,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY A B C D E\r\n"
@@ -82,7 +82,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP4 B C D E\r\n"
@@ -90,7 +90,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP4 1.2.3.4 C D E\r\n"
@@ -98,7 +98,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP4 1.2.3.4 D 1234 E\r\n"
@@ -106,7 +106,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP4 1.2.3.4 5.6.7.8 1234 E\r\n"
@@ -114,7 +114,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP6 B C D E\r\n"
@@ -122,7 +122,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP6 1:f::2 C D E\r\n"
@@ -130,7 +130,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP6 1:f::2 1234 D E\r\n"
@@ -138,7 +138,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP6 1:f::2 5:a::8 1234 E\r\n"
@@ -146,7 +146,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP4 1:f::2 5:a::8 1234 5678\r\n"
@@ -154,7 +154,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 {
 	send "PROXY TCP6 1.2.3.4 5.6.7.8 1234 5678\r\n"
@@ -162,7 +162,7 @@ client c1 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 # Finally try something which works...
 client c1 -proxy1 "1.2.3.4:1234 5.6.7.8:5678" {
@@ -179,7 +179,7 @@ client c1 -proxy1 "1.2.3.4:1234 5.6.7.8:5678" {
 	expect resp.http.rp != "1234"
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 client c1 -proxy1 "[1:f::2]:1234 [5:a::8]:5678" {
 	txreq -url /2
@@ -195,7 +195,7 @@ client c1 -proxy1 "[1:f::2]:1234 [5:a::8]:5678" {
 	expect resp.http.rp != "1234"
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 # Try with appended request (See also: #1728)
 client c2 {
@@ -204,13 +204,15 @@ client c2 {
 	expect resp.http.url == "/3"
 } -run
 
+varnish v1 -vsl_catchup
+
 # Malformed (missing \r)
 client c2 {
 	send "PROXY TCP4 1.2.3.4 5.6.7.8 1234 5678\n"
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 # Malformed, too long (106)
 # NB: Should check VSL for proper disposal
@@ -219,7 +221,7 @@ client c2 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 # Malformed, too long (107)
 # NB: Should check VSL for proper disposal
@@ -228,7 +230,7 @@ client c2 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 # Malformed, too long (108)
 # NB: Should check VSL for proper disposal
@@ -237,6 +239,6 @@ client c2 {
 	expect_close
 } -run
 
-delay .1
+varnish v1 -vsl_catchup
 
 logexpect l1 -wait
diff --git a/bin/varnishtest/vtc_client.c b/bin/varnishtest/vtc_client.c
index 5c714de0b..dac5a5840 100644
--- a/bin/varnishtest/vtc_client.c
+++ b/bin/varnishtest/vtc_client.c
@@ -32,6 +32,7 @@
 #include <sys/un.h>
 
 #include <errno.h>
+#include <netdb.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -73,21 +74,11 @@ static VTAILQ_HEAD(, client)	clients =
  * Send the proxy header
  */
 
-static int v_matchproto_(vss_resolved_f)
-proxy_cb(void *priv, const struct suckaddr *sa)
-{
-	struct suckaddr **addr = priv;
-	*addr = VSA_Clone(sa);
-	return (1);
-}
-
 static void
 client_proxy(struct vtclog *vl, int fd, int version, const char *spec)
 {
 	struct suckaddr *sac, *sas;
-	const char *err;
 	char *p, *p2;
-	int error;
 
 	p = strdup(spec);
 	AN(p);
@@ -95,14 +86,12 @@ client_proxy(struct vtclog *vl, int fd, int version, const char *spec)
 	AN(p2);
 	*p2++ = '\0';
 
-	error = VSS_resolver(p, NULL, proxy_cb, &sac, &err);
-	if (err != NULL)
-		vtc_fatal(vl, "Could not resolve client address: %s", err);
-	assert(error == 1);
-	error = VSS_resolver(p2, NULL, proxy_cb, &sas, &err);
-	if (err != NULL)
-		vtc_fatal(vl, "Could not resolve server address: %s", err);
-	assert(error == 1);
+	sac = VSS_ResolveOne(NULL, p, NULL, 0, SOCK_STREAM, AI_PASSIVE);
+	if (sac == NULL)
+		vtc_fatal(vl, "Could not resolve client address");
+	sas = VSS_ResolveOne(NULL, p2, NULL, 0, SOCK_STREAM, AI_PASSIVE);
+	if (sas == NULL)
+		vtc_fatal(vl, "Could not resolve server address");
 	if (vtc_send_proxy(fd, version, sac, sas))
 		vtc_fatal(vl, "Write failed: %s", strerror(errno));
 	free(p);
diff --git a/bin/varnishtest/vtc_main.c b/bin/varnishtest/vtc_main.c
index 01ae08ba7..ee9b74c10 100644
--- a/bin/varnishtest/vtc_main.c
+++ b/bin/varnishtest/vtc_main.c
@@ -39,6 +39,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/socket.h>
 
 #include "vtc.h"
 
@@ -433,18 +434,11 @@ i_mode(void)
  * Figure out what IP related magic
  */
 
-static int v_matchproto_(vss_resolved_f)
-bind_cb(void *priv, const struct suckaddr *sa)
-{
-	(void)priv;
-	return (VTCP_bind(sa, NULL));
-}
-
 static void
 ip_magic(void)
 {
-	const char *p;
 	int fd;
+	struct suckaddr *sa;
 	char abuf[VTCP_ADDRBUFSIZE];
 	char pbuf[VTCP_PORTBUFSIZE];
 
@@ -455,7 +449,9 @@ ip_magic(void)
 	 * XXX: "localhost", but that doesn't work out of the box.
 	 * XXX: Things like "prefer_ipv6" parameter complicates things.
 	 */
-	fd = VSS_resolver("127.0.0.1", NULL, bind_cb, NULL, &p);
+	sa = VSS_ResolveOne(NULL, "127.0.0.1", "0", 0, SOCK_STREAM, 0);
+	AN(sa);
+	fd = VTCP_bind(sa, NULL);
 	assert(fd >= 0);
 	VTCP_myname(fd, abuf, sizeof abuf, pbuf, sizeof(pbuf));
 	extmacro_def("localhost", "%s", abuf);
diff --git a/bin/varnishtest/vtc_misc.c b/bin/varnishtest/vtc_misc.c
index b55d4be13..8a1f80285 100644
--- a/bin/varnishtest/vtc_misc.c
+++ b/bin/varnishtest/vtc_misc.c
@@ -345,30 +345,19 @@ cmd_delay(CMD_ARGS)
  * DNS services.  This is a basic sanity check for those.
  */
 
-static int v_matchproto_(vss_resolved_f)
-dns_cb(void *priv, const struct suckaddr *sa)
+static int
+dns_works(void)
 {
+	struct suckaddr *sa;
 	char abuf[VTCP_ADDRBUFSIZE];
 	char pbuf[VTCP_PORTBUFSIZE];
-	int *ret = priv;
 
+	sa = VSS_ResolveOne(NULL, "dns-canary.freebsd.dk", NULL, 0, 0, 0);
+	if (sa == NULL)
+		return (0);
 	VTCP_name(sa, abuf, sizeof abuf, pbuf, sizeof pbuf);
-	if (strcmp(abuf, "192.0.2.255")) {
-		fprintf(stderr, "DNS-test: Wrong response: %s\n", abuf);
-		*ret = -1;
-	} else if (*ret == 0)
-		*ret = 1;
-	return (0);
-}
-
-static int
-dns_works(void)
-{
-	int ret = 0, error;
-	const char *msg;
-
-	error = VSS_resolver("dns-canary.freebsd.dk", NULL, dns_cb, &ret, &msg);
-	if (error || msg != NULL || ret != 1)
+	free(sa);
+	if (strcmp(abuf, "192.0.2.255"))
 		return (0);
 	return (1);
 }
diff --git a/include/vss.h b/include/vss.h
index aa645bdc7..ab4cdc33d 100644
--- a/include/vss.h
+++ b/include/vss.h
@@ -34,3 +34,6 @@ int VSS_resolver(const char *addr, const char *def_port, vss_resolved_f *func,
    void *priv, const char **err);
 int VSS_resolver_socktype(const char *addr, const char *def_port,
     vss_resolved_f *func, void *priv, const char **err, int socktype);
+struct suckaddr *VSS_ResolveOne(void *dst,
+    const char *addr, const char *port,
+    int family, int socktype, int flags);
diff --git a/lib/libvarnish/vss.c b/lib/libvarnish/vss.c
index 7282019de..38c766203 100644
--- a/lib/libvarnish/vss.c
+++ b/lib/libvarnish/vss.c
@@ -154,3 +154,42 @@ VSS_resolver(const char *addr, const char *def_port, vss_resolved_f *func,
 	return (VSS_resolver_socktype(
 	    addr, def_port, func, priv, err, SOCK_STREAM));
 }
+
+#include <stdio.h>
+
+struct suckaddr *
+VSS_ResolveOne(void *dst, const char *addr, const char *port,
+    int family, int socktype, int flags)
+{
+	struct addrinfo hints, *res = NULL;
+	struct suckaddr *retval = NULL;
+	char *p = NULL, *hp, *pp;
+	int error;
+
+	memset(&hints, 0, sizeof hints);
+	hints.ai_family = family;
+	hints.ai_socktype = socktype;
+	hints.ai_flags = flags;
+
+	AN(addr);
+	if (port != NULL) {
+		error = getaddrinfo(addr, port, &hints, &res);
+	} else {
+		p = strdup(addr);
+		AN(p);
+		if (vss_parse(p, &hp, &pp) != NULL || pp == NULL) {
+			free(p);
+			return (NULL);
+		}
+		error = getaddrinfo(hp, pp, &hints, &res);
+		free(p);
+	}
+	if (!error && res != NULL && res->ai_next == NULL) {
+		if (dst == NULL)
+			retval = VSA_Malloc(res->ai_addr, res->ai_addrlen);
+		else
+			retval = VSA_Build(dst, res->ai_addr, res->ai_addrlen);
+		freeaddrinfo(res);
+	}
+	return (retval);
+}
diff --git a/lib/libvmod_debug/vmod_debug_dyn.c b/lib/libvmod_debug/vmod_debug_dyn.c
index 338c2518c..35fdd9baa 100644
--- a/lib/libvmod_debug/vmod_debug_dyn.c
+++ b/lib/libvmod_debug/vmod_debug_dyn.c
@@ -40,6 +40,7 @@
 #include "cache/cache.h"
 
 #include "vsa.h"
+#include "vss.h"
 #include "vcc_if.h"
 
 struct xyzzy_debug_dyn {
@@ -62,7 +63,6 @@ static void
 dyn_dir_init(VRT_CTX, struct xyzzy_debug_dyn *dyn,
      VCL_STRING addr, VCL_STRING port, VCL_PROBE probe)
 {
-	struct addrinfo hints, *res = NULL;
 	struct suckaddr *sa;
 	struct director *dir, *dir2;
 	struct vrt_backend vrt;
@@ -77,13 +77,7 @@ dyn_dir_init(VRT_CTX, struct xyzzy_debug_dyn *dyn,
 	vrt.hosthdr = addr;
 	vrt.probe = probe;
 
-	memset(&hints, 0, sizeof(hints));
-	hints.ai_family = AF_UNSPEC;
-	hints.ai_socktype = SOCK_STREAM;
-	AZ(getaddrinfo(addr, port, &hints, &res));
-	XXXAZ(res->ai_next);
-
-	sa = VSA_Malloc(res->ai_addr, res->ai_addrlen);
+	sa = VSS_ResolveOne(NULL, addr, port, AF_UNSPEC, SOCK_STREAM, 0);
 	AN(sa);
 	if (VSA_Get_Proto(sa) == AF_INET) {
 		vrt.ipv4_addr = addr;
@@ -94,8 +88,6 @@ dyn_dir_init(VRT_CTX, struct xyzzy_debug_dyn *dyn,
 	} else
 		WRONG("Wrong proto family");
 
-	freeaddrinfo(res);
-
 	dir = VRT_new_backend(ctx, &vrt);
 	AN(dir);
 
diff --git a/lib/libvmod_std/vmod_std_conversions.c b/lib/libvmod_std/vmod_std_conversions.c
index ba10fa513..e98b98cef 100644
--- a/lib/libvmod_std/vmod_std_conversions.c
+++ b/lib/libvmod_std/vmod_std_conversions.c
@@ -41,6 +41,7 @@
 
 #include "vnum.h"
 #include "vsa.h"
+#include "vss.h"
 #include "vtim.h"
 #include "vcc_if.h"
 
@@ -79,13 +80,11 @@ vmod_integer(VRT_CTX, VCL_STRING p, VCL_INT i)
 VCL_IP
 vmod_ip(VRT_CTX, VCL_STRING s, VCL_IP d, VCL_BOOL n)
 {
-	struct addrinfo hints, *res0 = NULL;
-	const struct addrinfo *res;
-	int error;
 	void *p;
-	const struct suckaddr *r;
+	VCL_IP retval;
 
 	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
+	AN(s);
 	AN(d);
 	assert(VSA_Sane(d));
 
@@ -95,30 +94,14 @@ vmod_ip(VRT_CTX, VCL_STRING s, VCL_IP d, VCL_BOOL n)
 		    "vmod std.ip(): insufficient workspace");
 		return (d);
 	}
-	r = NULL;
-
-	if (s != NULL) {
-		memset(&hints, 0, sizeof(hints));
-		hints.ai_family = PF_UNSPEC;
-		hints.ai_socktype = SOCK_STREAM;
-		if (!n)
-			hints.ai_flags |= AI_NUMERICHOST;
-		error = getaddrinfo(s, "80", &hints, &res0);
-		if (!error) {
-			for (res = res0; res != NULL; res = res->ai_next) {
-				r = VSA_Build(p, res->ai_addr, res->ai_addrlen);
-				if (r != NULL)
-					break;
-			}
-		}
-	}
-	if (r == NULL) {
-		WS_Reset(ctx->ws, (uintptr_t)p);
-		r = d;
-	}
-	if (res0 != NULL)
-		freeaddrinfo(res0);
-	return (r);
+
+	retval = VSS_ResolveOne(p, s, "80", PF_UNSPEC, SOCK_STREAM,
+	    n ? 0 : AI_NUMERICHOST);
+	if (retval != NULL)
+		return (retval);
+
+	WS_Reset(ctx->ws, (uintptr_t)p);
+	return (d);
 }
 
 VCL_REAL v_matchproto_(td_std_real)


More information about the varnish-commit mailing list