[4.0] ed8b091 Reorganise querysort a bit

Federico G. Schwindt fgsch at lodoss.net
Tue Jun 24 11:31:57 CEST 2014


commit ed8b09192612cf22e85be226b966994e96cbdb72
Author: Federico G. Schwindt <fgsch at lodoss.net>
Date:   Sun Jun 22 09:31:02 2014 +0100

    Reorganise querysort a bit
    
    Also kill superflous function and test. Is this a candidate for its
    own file?

diff --git a/bin/varnishtest/tests/m00014.vtc b/bin/varnishtest/tests/m00014.vtc
index db2a62e..260280f 100644
--- a/bin/varnishtest/tests/m00014.vtc
+++ b/bin/varnishtest/tests/m00014.vtc
@@ -1,68 +1,36 @@
-varnishtest "Test querysort in std vmod"
-
-server s1 {
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
-
-    rxreq
-    txresp
+varnishtest "Test std.querysort"
 
+server s1 -repeat 5 {
+	rxreq
+	txresp
 } -start
 
 varnish v1 -vcl+backend {
+	import ${vmod_std};
 
-    import ${vmod_std};
-    sub vcl_deliver {
-        set resp.http.naren = std.querysort(req.url);
-    }
-
+	sub vcl_deliver {
+		set resp.http.url = std.querysort(req.url);
+	}
 } -start
 
 client c1 {
-
-    txreq -url "/video/44505073?title=0&byline=0&portrait=0&color=51a516"
-    rxresp
-    expect resp.http.naren == "/video/44505073?byline=0&color=51a516&portrait=0&title=0"
-
-    txreq -url "/video/44505073?byline=0&&&&&"
-    rxresp
-    expect resp.http.naren == "/video/44505073?byline=0"
-
-    txreq -url "/video/2?&"
-    rxresp
-    expect resp.http.naren == "/video/2?"
-
-    txreq -url "/video/2"
-    rxresp
-    expect resp.http.naren == "/video/2"
-
-    txreq -url "/video/2?cod=cape&cape=cod"
-    rxresp
-    expect resp.http.naren == "/video/2?cape=cod&cod=cape"
-
-    txreq -url "/"
-    rxresp
-    expect resp.http.naren == "/"
-
-}
-
-client c1 -run
+	txreq -url "/foo/bar?t=0&b=0&p=0&c=5"
+	rxresp
+	expect resp.http.url == "/foo/bar?b=0&c=5&p=0&t=0"
+
+	txreq -url "/foo/bar?coa=0&co=0"
+	rxresp
+	expect resp.http.url == "/foo/bar?co=0&coa=0"
+
+	txreq -url "/foo/bar?a=0&&&&&"
+	rxresp
+	expect resp.http.url == "/foo/bar?a=0"
+
+	txreq -url "/foo/bar?&"
+	rxresp
+	expect resp.http.url == "/foo/bar?"
+
+	txreq -url "/foo/bar"
+	rxresp
+	expect resp.http.url == "/foo/bar"
+} -run
diff --git a/bin/varnishtest/tests/m00015.vtc b/bin/varnishtest/tests/m00015.vtc
deleted file mode 100644
index 47109c8..0000000
--- a/bin/varnishtest/tests/m00015.vtc
+++ /dev/null
@@ -1,75 +0,0 @@
-varnishtest "Test querysort of req.url in vcl_hash"
-
-server s1 {
-
-    rxreq
-    txresp
-    rxreq
-    txresp
-
-} -start
-
-varnish v1 -vcl+backend {
-
-    import ${vmod_std};
-
-    sub vcl_hash {
-        set req.url = std.querysort(req.url);
-    }
-
-    sub vcl_deliver {
-        if (obj.hits > 0) {
-            set resp.http.X-Cache = "HIT";
-        }
-        else {
-            set resp.http.X-Cache = "MISS";
-        }
-    }
-
-} -start
-
-client c1 {
-
-    txreq -url "/video/47013255?title=0&byline=0&portrait=0&autoplay=1"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "MISS"
-
-    txreq -url "/video/47013255?title=0&byline=0&portrait=0&autoplay=1"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "HIT"
-
-}
-
-client c2 {
-
-    txreq -url "/video/47013255?autoplay=1&title=0&byline=0&portrait=0"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "HIT"
-
-    txreq -url "autoplay=1&title=0&byline=0&portrait=0&&&&"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "HIT"
-
-}
-
-client c2 {
-
-    txreq -url "/video/47013255?autoplay=1&title=0&byline=0&portrait=0"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "HIT"
-
-    txreq -url "/video/47013255?autoplay=1&title=0&byline=0&portrait=0&&&&"
-    rxresp
-    expect resp.status == 200
-    expect resp.http.X-Cache == "HIT"
-
-}
-
-
-client c1 -run
-client c2 -run
diff --git a/lib/libvmod_std/vmod_std.c b/lib/libvmod_std/vmod_std.c
index bedbd7d..60852b6 100644
--- a/lib/libvmod_std/vmod_std.c
+++ b/lib/libvmod_std/vmod_std.c
@@ -226,139 +226,97 @@ vmod_timestamp(const struct vrt_ctx *ctx, VCL_STRING label)
 	}
 }
 
+/*
+ * Boltsort
+ * Author: Naren Venkataraman of Vimeo Inc.
+ * Included here with permission.
+ */
 
-/* Boltsort
-   Author: Naren Venkataraman of Vimeo Inc.
+#define QS_MAX_PARAM_COUNT	32
+#define QS_EQUALS(a, b)	\
+    ((a) == (b) || ((a) == '\0' && (b) == '&') || ((a) == '&' && (b) == '\0'))
 
-   Included here with permission.
-*/
+static ssize_t
+param_compare(char *s, char *t)
+{
+	for (; QS_EQUALS(*s, *t); s++, t++) {
+		if (*s == '&' || s == '\0')
+			return (0);
+	}
+	return (*s - *t);
+}
 
+static size_t
+param_copy(char *dst, char *src)
+{
+	size_t len;
+	len = strcspn(src, "&");
+	memcpy(dst, src, len);
+	return (len);
+}
 
-#define QS_MAX_PARAM_COUNT 32
-#define QS_EQUALS(c, h) ((c == h) || (c == '\0' && h == '&') || (c == '&' && h == '\0'))
-#define QS_ENDS(s) (s == '&' || s == '\0') 
+VCL_STRING __match_proto__(td_std_querysort)
+vmod_querysort(const struct vrt_ctx *ctx, VCL_STRING url)
+{
+	char *param, *params[QS_MAX_PARAM_COUNT];
+	char *p, *r;
+	size_t len;
+	int param_count;
+	int i, n;
 
-static const char QS_TERMINATORS[2] = {'\0', '&'};
+	CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
 
-//since we dont store param length, we have to evaluate everytime
-static inline int param_compare (char *s, char *t)
-{
+	if (url == NULL)
+		return (NULL);
 
-    for ( ;QS_EQUALS(*s, *t); s++, t++) {
-        if (QS_ENDS(*s)) {
-            return 0;
-        }
-    }
-    return *s - *t;
+	p = strchr(url, '?');
+	if (p == NULL)
+		return (url);
 
-}
+	param_count = 0;
+	params[param_count++] = ++p;
+	len = p - url;
 
-//end of param is either first occurance of & or '\0'
-static inline int param_copy(char *dst, char *src, char *last_param)
-{
+	while ((p = strchr(p, '&')) != NULL) {
+		param = ++p;
 
-    int len = strchr(src, QS_TERMINATORS[(src != last_param)]) - src;
-    memcpy(dst, src, len);
-    return len;
+		for (i = 0; i < param_count; i++) {
+			if (param[0] < params[i][0] ||
+			    param_compare(param, params[i]) < 0) {
+				for (n = param_count; n > i; n--)
+					params[n] = params[n - 1];
+				break;
+			}
+		}
+		params[i] = param;
+		param_count++;
 
-}
+		if (param_count == QS_MAX_PARAM_COUNT)
+			return (url);
+	}
 
-//Varnish vmod requires this
-int init_function(struct vmod_priv *priv, const struct VCL_conf *conf)
-{
-    return 0;
+	if (param_count == 1)
+		return (url);
 
-}
+	r = WS_Alloc(ctx->ws, strchr(param, '\0') - url + 1);
+	if (r == NULL)
+		return (url);
 
-//sort query string
-VCL_STRING vmod_querysort(const struct vrt_ctx * ctx, VCL_STRING url)
-{
+	p = memcpy(r, url, len);
+	p += len;
+
+	for (i = 0; i < param_count - 1; i++) {
+		if (params[i][0] != '\0' && params[i][0] != '&')
+			break;
+	}
+
+	for (; i < param_count - 1; i++) {
+		p += param_copy(p, params[i]);
+		*p++ = '&';
+	}
 
-    if (url == NULL) {
-        return NULL;
-    }
-
-    int qs_index = 0;
-    int param_count = 0;
-
-    char *dst_url = NULL;
-    char *qs = NULL;
-
-    //To avoid 1 pass for count calculations, assuming MAX_PARAM_COUNT as max
-    char* params[QS_MAX_PARAM_COUNT];
-
-    int i, p;
-    char *param = NULL;
-
-    qs = strchr(url, '?');
-    if(!qs) {
-        return url;
-    }
-
-    //add first param and increment count
-    params[param_count++] = ++qs;
-    qs_index = qs - url;
-
-    //Continue to find query string
-    while((qs = strchr(qs, '&')) != NULL) {
-        param = ++qs;
-
-        for(p = 0; p < param_count; p++) {
-            //if incoming param is < param at position then place it at p and then move up rest
-            if(param[0] < params[p][0] || param_compare(param, params[p]) < 0) {
-                for(i = param_count; i > p; i--) {
-                    params[i] = params[i-1];
-                }
-                break;
-            }
-        }
-        params[p] = param;
-        param_count++;
-
-        //if it exceed max params return as is
-        if (param_count == QS_MAX_PARAM_COUNT) {
-            return url;
-        }
-    }
-
-    //there is nothing after & 
-    //eg: http://127.0.0.1/?me=1&
-    if (param_count == 1) {
-        return url;
-    }
-
-    //allocate space for sorted url
-    //    struct ws *ws = sp->wrk->ws; 
-    struct ws *ws = ctx->ws;
-    dst_url = WS_Alloc(ws, strchr(param, '\0') - url + 1);
-    WS_Assert(ws);
-
-    //if alloc fails return as is
-    if(dst_url == NULL) {
-        return url;
-    }
-
-    //copy data before query string
-    char* cp = memcpy(dst_url, url, qs_index) + qs_index;
-
-    //get rid of all empty params /test/?a&&&
-    for(p = 0; p < param_count - 1; p++) { 
-        if (params[p][0] != '\0' && params[p][0] != '&') {
-            break;
-        }
-    }
-
-    //copy sorted params
-    for(; p < param_count - 1; p++) {
-        //copy and increment
-        cp += param_copy(cp, params[p], param);
-        *cp++ = '&';
-    }
-
-    //copy the last param
-    cp += param_copy(cp, params[p], param);
-    *cp = '\0';
-
-    return dst_url;
+	p += param_copy(p, params[i]);
+	*p = '\0';
 
+	return (r);
 }



More information about the varnish-commit mailing list