[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