[master] 9867631 Prevent storage backends name collisions
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Wed May 3 00:11:05 CEST 2017
commit 98676319878b7a6aca77be7e207e140202800ae1
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Tue May 2 23:57:15 2017 +0200
Prevent storage backends name collisions
Because of Transient storage being a special-case stevedore, the
STV_Foreach logic would only work after the child's startup once
all storage backends are defined. So during the setup, collisions
would only be detected for the Transient storage.
The underlying STV__iter function is now pretty much like a plain
VTAILQ_FOREACH loop with an additional CHECK_OBJ_ORNULL step on all
iterations.
As a result, the transient storage is appended to the list at the
end of the manager's setup and is now reported last in the CLI's
storage.list command.
Error messages related to the -s option are slightly more helpful.
Fixes #2321
diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index 304b3bd..1ddf8a5 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -57,17 +57,11 @@ STV__iter(struct stevedore ** const pp)
AN(pp);
CHECK_OBJ_ORNULL(*pp, STEVEDORE_MAGIC);
- if (*pp == stv_transient) {
- *pp = NULL;
- return (0);
- }
if (*pp != NULL)
*pp = VTAILQ_NEXT(*pp, list);
else
*pp = VTAILQ_FIRST(&stevedores);
- if (*pp == NULL)
- *pp = stv_transient;
- return (1);
+ return (*pp != NULL);
}
/*--------------------------------------------------------------------*/
@@ -128,6 +122,26 @@ static const struct choice STV_choice[] = {
{ NULL, NULL }
};
+static void
+stv_check_ident(const char *spec, const char *ident)
+{
+ struct stevedore *stv;
+ unsigned found = 0;
+
+ if (!strcmp(ident, TRANSIENT_STORAGE))
+ found = (stv_transient != NULL);
+ else {
+ STV_Foreach(stv)
+ if (!strcmp(stv->ident, ident)) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (found)
+ ARGV_ERR("(-s %s) '%s' is already defined\n", spec, ident);
+}
+
void
STV_Config(const char *spec)
{
@@ -135,7 +149,6 @@ STV_Config(const char *spec)
const char *p, *q;
struct stevedore *stv;
const struct stevedore *stv2;
- struct stevedore *stv3;
int ac, l;
static unsigned seq = 0;
@@ -183,15 +196,12 @@ STV_Config(const char *spec)
bprintf(stv->ident, "%.*s", l, spec);
}
- STV_Foreach(stv3)
- if (!strcmp(stv3->ident, stv->ident))
- ARGV_ERR("(-s%s=%s) already defined once\n",
- stv->ident, stv->name);
+ stv_check_ident(spec, stv->ident);
if (stv->init != NULL)
stv->init(stv, ac, av);
else if (ac != 0)
- ARGV_ERR("(-s%s) too many arguments\n", stv->name);
+ ARGV_ERR("(-s %s) too many arguments\n", stv->name);
AN(stv->allocobj);
AN(stv->methods);
@@ -199,9 +209,8 @@ STV_Config(const char *spec)
if (!strcmp(stv->ident, TRANSIENT_STORAGE)) {
AZ(stv_transient);
stv_transient = stv;
- } else {
+ } else
VTAILQ_INSERT_TAIL(&stevedores, stv, list);
- }
/* NB: Do not free av, stevedore gets to keep it */
}
@@ -216,4 +225,5 @@ STV_Config_Transient(void)
VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_stv);
if (stv_transient == NULL)
STV_Config(TRANSIENT_STORAGE "=malloc");
+ VTAILQ_INSERT_TAIL(&stevedores, stv_transient, list);
}
diff --git a/bin/varnishtest/tests/r02321.vtc b/bin/varnishtest/tests/r02321.vtc
new file mode 100644
index 0000000..33c7658
--- /dev/null
+++ b/bin/varnishtest/tests/r02321.vtc
@@ -0,0 +1,19 @@
+varnishtest "Storage name collisions"
+
+# intentional collision
+shell -err -expect "Error: (-s main=malloc,100m) 'main' is already defined" {
+ exec varnishd -a :0 -f '' -n ${tmpdir} \
+ -s main=malloc,10m -s main=malloc,100m
+}
+
+# pseudo-accidental collision
+shell -err -expect "Error: (-s s0=malloc,100m) 's0' is already defined" {
+ exec varnishd -a :0 -f '' -n ${tmpdir} \
+ -s malloc,10m -s s0=malloc,100m
+}
+
+# Transient collision
+shell -err -expect "Error: (-s Transient=malloc,100m) 'Transient' is already defined" {
+ exec varnishd -a :0 -f '' -n ${tmpdir} \
+ -s Transient=malloc,10m -s Transient=malloc,100m
+}
More information about the varnish-commit
mailing list