[experimental-ims] ccf9f85 The CLI protocol and its implementation is not designed for transferring huge amounts of data, and it should not be used that way.
Geoff Simmons
geoff at varnish-cache.org
Mon Jan 9 21:52:33 CET 2012
commit ccf9f85a217bb240550e9c2fac69c25ab23d8187
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Nov 14 10:18:18 2011 +0000
The CLI protocol and its implementation is not designed for
transferring huge amounts of data, and it should not be used that
way.
But we have commands like ban.list which potentially can return at
metric shitload of data and this is a problem.
This commit implements a cli_limit paramter which (with its upper
limit) protects from responses that cannot be moved by the CLI
protocol no matter what.
We set the default to a low 4096 bytes, in order to make people
specifically shoot their feet off if they really want to see 200k
bans.
diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index f29f86a..f299bef 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -236,7 +236,8 @@ CLI_Init(void)
Lck_New(&cli_mtx, lck_cli);
cli_thread = pthread_self();
- cls = VCLS_New(cli_cb_before, cli_cb_after, cache_param->cli_buffer);
+ cls = VCLS_New(cli_cb_before, cli_cb_after,
+ &cache_param->cli_buffer, &cache_param->cli_limit);
AN(cls);
CLI_AddFuncs(master_cmds);
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index fb71e33..ee8bad2 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -107,6 +107,7 @@ struct params {
/* CLI related */
unsigned cli_timeout;
+ unsigned cli_limit;
unsigned ping_interval;
/* LRU list ordering interval */
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 70e8557..1d46d82 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -336,7 +336,8 @@ static void
mgt_cli_init_cls(void)
{
- cls = VCLS_New(mgt_cli_cb_before, mgt_cli_cb_after, mgt_param.cli_buffer);
+ cls = VCLS_New(mgt_cli_cb_before, mgt_cli_cb_after,
+ &mgt_param.cli_buffer, &mgt_param.cli_limit);
AN(cls);
AZ(VCLS_AddFunc(cls, MCF_NOAUTH, cli_auth));
AZ(VCLS_AddFunc(cls, MCF_AUTH, cli_proto));
diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index 981841b..8a9e1fc 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -349,6 +349,7 @@ main(int argc, char * const *argv)
struct cli cli[1];
struct vpf_fh *pfh = NULL;
char *dirname;
+ unsigned clilim;
/*
* Start out by closing all unwanted file descriptors we might
@@ -389,9 +390,12 @@ main(int argc, char * const *argv)
SHA256_Test();
memset(cli, 0, sizeof cli);
+ cli[0].magic = CLI_MAGIC;
cli[0].sb = VSB_new_auto();
XXXAN(cli[0].sb);
cli[0].result = CLIS_OK;
+ clilim = 32768;
+ cli[0].limit = &clilim;
VTAILQ_INIT(&heritage.socks);
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index 8050d08..afaab84 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -673,6 +673,19 @@ static const struct parspec input_parspec[] = {
"Listen queue depth.",
MUST_RESTART,
"1024", "connections" },
+ { "cli_buffer", tweak_uint, &mgt_param.cli_buffer, 4096, UINT_MAX,
+ "Size of buffer for CLI command input."
+ "\nYou may need to increase this if you have big VCL files "
+ "and use the vcl.inline CLI command.\n"
+ "NB: Must be specified with -p to have effect.\n",
+ 0,
+ "8192", "bytes" },
+ { "cli_limit", tweak_uint, &mgt_param.cli_limit, 128, 99999999,
+ "Maximum size of CLI response. If the response exceeds"
+ " this limit, the reponse code will be 201 instead of"
+ " 200 and the last line will indicate the truncation.",
+ 0,
+ "4096", "bytes" },
{ "cli_timeout", tweak_timeout, &mgt_param.cli_timeout, 0, 0,
"Timeout for the childs replies to CLI requests from "
"the mgt_param.",
@@ -805,13 +818,6 @@ static const struct parspec input_parspec[] = {
"more sessions take a detour around the waiter.",
EXPERIMENTAL,
"50", "ms" },
- { "cli_buffer", tweak_uint, &mgt_param.cli_buffer, 4096, UINT_MAX,
- "Size of buffer for CLI input."
- "\nYou may need to increase this if you have big VCL files "
- "and use the vcl.inline CLI command.\n"
- "NB: Must be specified with -p to have effect.\n",
- 0,
- "8192", "bytes" },
{ "log_hashstring", tweak_bool, &mgt_param.log_hash, 0, 0,
"Log the hash string components to shared memory log.\n",
0,
diff --git a/bin/varnishtest/tests/b00008.vtc b/bin/varnishtest/tests/b00008.vtc
index 1e9aa9d..cee8d4c 100644
--- a/bin/varnishtest/tests/b00008.vtc
+++ b/bin/varnishtest/tests/b00008.vtc
@@ -33,3 +33,7 @@ varnish v1 -cliok "help"
varnish v1 -cliok "param.set waiter default"
varnish v1 -clierr 106 "param.set waiter HASH(0x8839c4c)"
+
+varnish v1 -cliok "param.set cli_limit 128"
+
+varnish v1 -clierr 201 "param.show"
diff --git a/include/vcli.h b/include/vcli.h
index cd98f58..04ac11c 100644
--- a/include/vcli.h
+++ b/include/vcli.h
@@ -199,6 +199,7 @@ enum VCLI_status_e {
CLIS_PARAM = 106,
CLIS_AUTH = 107,
CLIS_OK = 200,
+ CLIS_TRUNCATED = 201,
CLIS_CANT = 300,
CLIS_COMMS = 400,
CLIS_CLOSE = 500
diff --git a/include/vcli_common.h b/include/vcli_common.h
index 80d680f..8d0a623 100644
--- a/include/vcli_common.h
+++ b/include/vcli_common.h
@@ -42,4 +42,5 @@ struct cli {
char *ident;
struct vlu *vlu;
struct VCLS *cls;
+ volatile unsigned *limit;
};
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index 0ac29f8..7e4e6e4 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -30,7 +30,8 @@
struct VCLS;
typedef void cls_cb_f(void *priv);
typedef void cls_cbc_f(const struct cli*);
-struct VCLS *VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen);
+struct VCLS *VCLS_New(cls_cbc_f *before, cls_cbc_f *after,
+ volatile unsigned *maxlen, volatile unsigned *limit);
struct cli *VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc,
void *priv);
int VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp);
diff --git a/lib/libvarnish/cli_common.c b/lib/libvarnish/cli_common.c
index 4a17de5..7b09bdb 100644
--- a/lib/libvarnish/cli_common.c
+++ b/lib/libvarnish/cli_common.c
@@ -43,6 +43,7 @@
#include <time.h>
#include <unistd.h>
+#include "miniobj.h"
#include "vas.h"
#include "vcli.h"
#include "vcli_common.h"
@@ -56,10 +57,15 @@ VCLI_Out(struct cli *cli, const char *fmt, ...)
va_list ap;
va_start(ap, fmt);
- if (cli != NULL)
- (void)VSB_vprintf(cli->sb, fmt, ap);
- else
+ if (cli != NULL) {
+ CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
+ if (VSB_len(cli->sb) < *cli->limit)
+ (void)VSB_vprintf(cli->sb, fmt, ap);
+ else if (cli->result == CLIS_OK)
+ cli->result = CLIS_TRUNCATED;
+ } else {
(void)vfprintf(stdout, fmt, ap);
+ }
va_end(ap);
}
@@ -68,6 +74,7 @@ void
VCLI_Quote(struct cli *cli, const char *s)
{
+ CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
VSB_quote(cli->sb, s, -1, 0);
}
@@ -75,10 +82,13 @@ void
VCLI_SetResult(struct cli *cli, unsigned res)
{
- if (cli != NULL)
- cli->result = res; /*lint !e64 type mismatch */
- else
+ if (cli != NULL) {
+ CHECK_OBJ_NOTNULL(cli, CLI_MAGIC);
+ if (cli->result != CLIS_TRUNCATED || res != CLIS_OK)
+ cli->result = res; /*lint !e64 type mismatch */
+ } else {
printf("CLI result = %u\n", res);
+ }
}
int
diff --git a/lib/libvarnish/cli_serve.c b/lib/libvarnish/cli_serve.c
index f3def96..547061c 100644
--- a/lib/libvarnish/cli_serve.c
+++ b/lib/libvarnish/cli_serve.c
@@ -81,7 +81,8 @@ struct VCLS {
unsigned nfd;
VTAILQ_HEAD(,VCLS_func) funcs;
cls_cbc_f *before, *after;
- unsigned maxlen;
+ volatile unsigned *maxlen;
+ volatile unsigned *limit;
};
/*--------------------------------------------------------------------*/
@@ -245,6 +246,10 @@ cls_vlu2(void *priv, char * const *av)
struct VCLS_func *cfn;
struct cli *cli;
unsigned na;
+ ssize_t len;
+ char *s;
+ unsigned lim;
+ const char *trunc = "!\n[response was truncated]\n";
CAST_OBJ_NOTNULL(cfd, priv, VCLS_FD_MAGIC);
cs = cfd->cls;
@@ -297,7 +302,16 @@ cls_vlu2(void *priv, char * const *av)
cli->cls = NULL;
- if (VCLI_WriteResult(cfd->fdo, cli->result, VSB_data(cli->sb)) ||
+ s = VSB_data(cli->sb);
+ len = VSB_len(cli->sb);
+ lim = *cs->limit;
+ if (len > lim) {
+ if (cli->result == CLIS_OK)
+ cli->result = CLIS_TRUNCATED;
+ strcpy(s + (lim - strlen(trunc)), trunc);
+ assert(strlen(s) <= lim);
+ }
+ if (VCLI_WriteResult(cfd->fdo, cli->result, s) ||
cli->result == CLIS_CLOSE)
return (1);
@@ -380,7 +394,8 @@ cls_vlu(void *priv, const char *p)
}
struct VCLS *
-VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen)
+VCLS_New(cls_cbc_f *before, cls_cbc_f *after, volatile unsigned *maxlen,
+ volatile unsigned *limit)
{
struct VCLS *cs;
@@ -391,6 +406,7 @@ VCLS_New(cls_cbc_f *before, cls_cbc_f *after, unsigned maxlen)
cs->before = before;
cs->after = after;
cs->maxlen = maxlen;
+ cs->limit = limit;
return (cs);
}
@@ -409,8 +425,9 @@ VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc, void *priv)
cfd->fdo = fdo;
cfd->cli = &cfd->clis;
cfd->cli->magic = CLI_MAGIC;
- cfd->cli->vlu = VLU_New(cfd, cls_vlu, cs->maxlen);
+ cfd->cli->vlu = VLU_New(cfd, cls_vlu, *cs->maxlen);
cfd->cli->sb = VSB_new_auto();
+ cfd->cli->limit = cs->limit;
cfd->closefunc = closefunc;
cfd->priv = priv;
AN(cfd->cli->sb);
More information about the varnish-commit
mailing list