[master] 3e254078a vre: Extract VRE_error() from VRE_compile()
Dridi Boukelmoune
dridi.boukelmoune at gmail.com
Mon Jul 5 15:49:05 UTC 2021
commit 3e254078a52ff9f271e995e64ca5d2b6443bae04
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Fri Jun 18 10:08:26 2021 +0200
vre: Extract VRE_error() from VRE_compile()
This is a major step back in terms of error reporting, but I haven't
found anything in libpcre to translate an error code to an error
message. "pcre error %d" should still be a good enough hint.
Looking forward, we are going to need this for libpcre2 but again the
API leaves some to be desired and only works by writing error messages
to buffers.
The return value implies that we got a valid error code, which will be
verified with pcre2. I couldn't find how to verify error codes with pcre.
Writing to the VSB is however fail-safe, it's the caller's job to deal
with a VSB error.
diff --git a/bin/varnishd/cache/cache_vrt_re.c b/bin/varnishd/cache/cache_vrt_re.c
index bf58e1f71..bda5256f6 100644
--- a/bin/varnishd/cache/cache_vrt_re.c
+++ b/bin/varnishd/cache/cache_vrt_re.c
@@ -42,8 +42,7 @@ void
VPI_re_init(vre_t **rep, const char *re)
{
vre_t *t;
- const char *error;
- int erroroffset;
+ int error, erroroffset;
/* This was already check-compiled by the VCL compiler */
t = VRE_compile(re, 0, &error, &erroroffset);
diff --git a/bin/varnishtest/tests/u00006.vtc b/bin/varnishtest/tests/u00006.vtc
index c331d7ddf..b2d7dcef9 100644
--- a/bin/varnishtest/tests/u00006.vtc
+++ b/bin/varnishtest/tests/u00006.vtc
@@ -44,7 +44,7 @@ shell -err -expect {-I: "foo" matches zero tags} \
"varnishlog -I foo:bar"
shell -err -expect {-I: "Resp" is ambiguous} \
"varnishlog -I Resp:bar"
-shell -err -expect {-I: Regex error at position 4 (missing ))} \
+shell -err -expect {-I: Regex error at position 4 (pcre error 14)} \
{varnishlog -I "(foo"}
shell -err -expect "-t: Invalid argument" \
"varnishlog -t -1"
diff --git a/bin/varnishtest/vtc_haproxy.c b/bin/varnishtest/vtc_haproxy.c
index f54b9c04b..ec6ab1b40 100644
--- a/bin/varnishtest/vtc_haproxy.c
+++ b/bin/varnishtest/vtc_haproxy.c
@@ -298,10 +298,10 @@ static void v_matchproto_(cmd_f)
cmd_haproxy_cli_expect(CMD_ARGS)
{
struct haproxy_cli *hc;
+ struct vsb vsb[1];
vre_t *vre;
- const char *error;
- int erroroffset, i, ret;
- char *cmp, *spec;
+ int error, erroroffset, i, ret;
+ char *cmp, *spec, errbuf[VRE_ERROR_LEN];
(void)vl;
CAST_OBJ_NOTNULL(hc, priv, HAPROXY_CLI_MAGIC);
@@ -319,9 +319,14 @@ cmd_haproxy_cli_expect(CMD_ARGS)
haproxy_cli_recv(hc);
vre = VRE_compile(spec, 0, &error, &erroroffset);
- if (!vre)
+ if (vre == NULL) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, error));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
vtc_fatal(hc->vl, "CLI regexp error: '%s' (@%d) (%s)",
- error, erroroffset, spec);
+ errbuf, erroroffset, spec);
+ }
i = VRE_match(vre, hc->rxbuf, 0, 0, NULL);
diff --git a/bin/varnishtest/vtc_logexp.c b/bin/varnishtest/vtc_logexp.c
index 152144021..a1d8a2c55 100644
--- a/bin/varnishtest/vtc_logexp.c
+++ b/bin/varnishtest/vtc_logexp.c
@@ -624,13 +624,11 @@ static void
cmd_logexp_common(struct logexp *le, struct vtclog *vl,
int skip_max, char * const *av)
{
- int vxid;
- int tag;
vre_t *vre;
- const char *err;
- int pos;
+ struct vsb vsb[1];
+ int err, pos, tag, vxid;
struct logexp_test *test;
- char *end;
+ char *end, errbuf[VRE_ERROR_LEN];
if (!strcmp(av[2], "*"))
vxid = LE_ANY;
@@ -653,9 +651,14 @@ cmd_logexp_common(struct logexp *le, struct vtclog *vl,
vre = NULL;
if (av[4]) {
vre = VRE_compile(av[4], 0, &err, &pos);
- if (vre == NULL)
+ if (vre == NULL) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, err));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
vtc_fatal(vl, "Regex error (%s): '%s' pos %d",
- err, av[4], pos);
+ errbuf, av[4], pos);
+ }
}
ALLOC_OBJ(test, LOGEXP_TEST_MAGIC);
diff --git a/bin/varnishtest/vtc_misc.c b/bin/varnishtest/vtc_misc.c
index a2b2038c7..4d267c46b 100644
--- a/bin/varnishtest/vtc_misc.c
+++ b/bin/varnishtest/vtc_misc.c
@@ -143,23 +143,28 @@ static void
cmd_shell_engine(struct vtclog *vl, int ok, const char *cmd,
const char *expect, const char *re)
{
- struct vsb *vsb;
+ struct vsb *vsb, re_vsb[1];
FILE *fp;
vre_t *vre = NULL;
- const char *errptr;
int r, c;
- int err;
+ int err, erroff;
+ char errbuf[VRE_ERROR_LEN];
AN(vl);
AN(cmd);
vsb = VSB_new_auto();
AN(vsb);
if (re != NULL) {
- vre = VRE_compile(re, 0, &errptr, &err);
- if (vre == NULL)
+ vre = VRE_compile(re, 0, &err, &erroff);
+ if (vre == NULL) {
+ AN(VSB_init(re_vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(re_vsb, err));
+ AZ(VSB_finish(re_vsb));
+ VSB_fini(re_vsb);
vtc_fatal(vl,
"shell_match invalid regexp (\"%s\" at %d)",
- errptr, err);
+ errbuf, erroff);
+ }
}
VSB_printf(vsb, "exec 2>&1 ; %s", cmd);
AZ(VSB_finish(vsb));
diff --git a/bin/varnishtest/vtc_subr.c b/bin/varnishtest/vtc_subr.c
index 7d5607cd0..3d78566e7 100644
--- a/bin/varnishtest/vtc_subr.c
+++ b/bin/varnishtest/vtc_subr.c
@@ -87,10 +87,11 @@ vtc_expect(struct vtclog *vl,
const char *orhs, const char *rhs)
{
vre_t *vre;
- const char *error;
- int erroroffset;
+ struct vsb vsb[1];
+ int error, erroroffset;
int i, j, retval = -1;
double fl, fr;
+ char errbuf[VRE_ERROR_LEN];
j = lhs == NULL || rhs == NULL;
if (lhs == NULL)
@@ -100,9 +101,14 @@ vtc_expect(struct vtclog *vl,
if (!strcmp(cmp, "~") || !strcmp(cmp, "!~")) {
vre = VRE_compile(rhs, 0, &error, &erroroffset);
- if (vre == NULL)
+ if (vre == NULL) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, error));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
vtc_fatal(vl, "REGEXP error: %s (@%d) (%s)",
- error, erroroffset, rhs);
+ errbuf, erroroffset, rhs);
+ }
i = VRE_match(vre, lhs, 0, 0, NULL);
retval = (i >= 0 && *cmp == '~') || (i < 0 && *cmp == '!');
VRE_free(&vre);
diff --git a/bin/varnishtest/vtc_syslog.c b/bin/varnishtest/vtc_syslog.c
index 221a5b6f5..78ccf77ee 100644
--- a/bin/varnishtest/vtc_syslog.c
+++ b/bin/varnishtest/vtc_syslog.c
@@ -388,10 +388,10 @@ static void v_matchproto_(cmd_f)
cmd_syslog_expect(CMD_ARGS)
{
struct syslog_srv *s;
+ struct vsb vsb[1];
vre_t *vre;
- const char *error;
- int erroroffset, i, ret;
- char *cmp, *spec;
+ int error, erroroffset, i, ret;
+ char *cmp, *spec, errbuf[VRE_ERROR_LEN];
(void)vl;
CAST_OBJ_NOTNULL(s, priv, SYSLOG_SRV_MAGIC);
@@ -407,9 +407,14 @@ cmd_syslog_expect(CMD_ARGS)
assert(!strcmp(cmp, "~") || !strcmp(cmp, "!~"));
vre = VRE_compile(spec, 0, &error, &erroroffset);
- if (!vre)
+ if (vre == NULL) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, error));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
vtc_fatal(s->vl, "REGEXP error: '%s' (@%d) (%s)",
- error, erroroffset, spec);
+ errbuf, erroroffset, spec);
+ }
i = VRE_match(vre, s->rxbuf, 0, 0, NULL);
diff --git a/bin/varnishtest/vtc_varnish.c b/bin/varnishtest/vtc_varnish.c
index 97a876876..a463b5665 100644
--- a/bin/varnishtest/vtc_varnish.c
+++ b/bin/varnishtest/vtc_varnish.c
@@ -739,16 +739,22 @@ static void
varnish_cli(struct varnish *v, const char *cli, unsigned exp, const char *re)
{
enum VCLI_status_e u;
+ struct vsb vsb[1];
vre_t *vre = NULL;
- char *resp = NULL;
- const char *errptr;
- int err;
+ char *resp = NULL, errbuf[VRE_ERROR_LEN];
+ int err, erroff;
VARNISH_LAUNCH(v);
if (re != NULL) {
- vre = VRE_compile(re, 0, &errptr, &err);
- if (vre == NULL)
- vtc_fatal(v->vl, "Illegal regexp");
+ vre = VRE_compile(re, 0, &err, &erroff);
+ if (vre == NULL) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, err));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
+ vtc_fatal(v->vl, "Illegal regexp: %s (@%d)",
+ errbuf, erroff);
+ }
}
u = varnish_ask_cli(v, cli, &resp);
vtc_log(v->vl, 2, "CLI %03u <%s>", u, cli);
@@ -756,8 +762,13 @@ varnish_cli(struct varnish *v, const char *cli, unsigned exp, const char *re)
vtc_fatal(v->vl, "FAIL CLI response %u expected %u", u, exp);
if (vre != NULL) {
err = VRE_match(vre, resp, 0, 0, NULL);
- if (err < 1)
- vtc_fatal(v->vl, "Expect failed (%d)", err);
+ if (err < 1) {
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, err));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
+ vtc_fatal(v->vl, "Expect failed (%s)", errbuf);
+ }
VRE_free(&vre);
}
free(resp);
diff --git a/include/vre.h b/include/vre.h
index 488791daf..0ecdfe806 100644
--- a/include/vre.h
+++ b/include/vre.h
@@ -37,6 +37,8 @@
#ifndef VRE_H_INCLUDED
#define VRE_H_INCLUDED
+#define VRE_ERROR_LEN 128
+
struct vre;
struct vsb;
@@ -54,7 +56,8 @@ typedef struct vre vre_t;
extern const unsigned VRE_has_jit;
extern const unsigned VRE_CASELESS;
-vre_t *VRE_compile(const char *, unsigned, const char **, int *);
+vre_t *VRE_compile(const char *, unsigned, int *, int *);
+int VRE_error(struct vsb *, int err);
int VRE_match(const vre_t *code, const char *subject, size_t length,
int options, const volatile struct vre_limits *lim);
int VRE_sub(const vre_t *code, const char *subject, const char *replacement,
diff --git a/lib/libvarnish/vre.c b/lib/libvarnish/vre.c
index 5dae04f97..006b15afb 100644
--- a/lib/libvarnish/vre.c
+++ b/lib/libvarnish/vre.c
@@ -32,6 +32,7 @@
#include <pcre.h>
#include <ctype.h>
+#include <stdio.h>
#include <string.h>
#include <unistd.h>
@@ -80,24 +81,34 @@ const unsigned VRE_CASELESS = PCRE_CASELESS;
vre_t *
VRE_compile(const char *pattern, unsigned options,
- const char **errptr, int *erroffset)
+ int *errptr, int *erroffset)
{
+ const char *errstr = NULL;
vre_t *v;
- *errptr = NULL; *erroffset = 0;
+
+ AN(pattern);
+ AZ(options & (~VRE_MASK_COMPILE));
+ AN(errptr);
+ AN(erroffset);
+
+ *errptr = 0;
+ *erroffset = -1;
ALLOC_OBJ(v, VRE_MAGIC);
if (v == NULL) {
- *errptr = "Out of memory for VRE";
+ *errptr = PCRE_ERROR_NOMEMORY;
return (NULL);
}
AZ(options & (~VRE_MASK_COMPILE));
- v->re = pcre_compile(pattern, options, errptr, erroffset, NULL);
+ v->re = pcre_compile2(pattern, options, errptr, &errstr, erroffset,
+ NULL);
if (v->re == NULL) {
VRE_free(&v);
return (NULL);
}
- v->re_extra = pcre_study(v->re, VRE_STUDY_JIT_COMPILE, errptr);
- if (*errptr != NULL) {
+ v->re_extra = pcre_study(v->re, VRE_STUDY_JIT_COMPILE, &errstr);
+ if (errstr != NULL) {
+ *errptr = PCRE_ERROR_INTERNAL;
VRE_free(&v);
return (NULL);
}
@@ -106,7 +117,7 @@ VRE_compile(const char *pattern, unsigned options,
v->re_extra = calloc(1, sizeof(pcre_extra));
v->my_extra = 1;
if (v->re_extra == NULL) {
- *errptr = "Out of memory for pcre_extra";
+ *errptr = PCRE_ERROR_NOMEMORY;
VRE_free(&v);
return (NULL);
}
@@ -114,6 +125,15 @@ VRE_compile(const char *pattern, unsigned options,
return (v);
}
+int
+VRE_error(struct vsb *vsb, int err)
+{
+
+ CHECK_OBJ_NOTNULL(vsb, VSB_MAGIC);
+ VSB_printf(vsb, "pcre error %d", err);
+ return (0);
+}
+
static int
vre_exec(const vre_t *code, const char *subject, int length,
int startoffset, int options, int *ovector, int ovecsize,
diff --git a/lib/libvarnishapi/vsl_arg.c b/lib/libvarnishapi/vsl_arg.c
index 4a8ee9a06..d42d92b24 100644
--- a/lib/libvarnishapi/vsl_arg.c
+++ b/lib/libvarnishapi/vsl_arg.c
@@ -46,6 +46,7 @@
#include "vnum.h"
#include "vqueue.h"
#include "vre.h"
+#include "vsb.h"
#include "vapi/vsl.h"
@@ -245,11 +246,14 @@ vsl_ix_arg(struct VSL_data *vsl, int opt, const char *arg)
static int
vsl_IX_arg(struct VSL_data *vsl, int opt, const char *arg)
{
- int i, l, off;
- const char *b, *e, *err;
+ int i, l, off, err;
+ const char *b, *e;
vre_t *vre;
+ struct vsb vsb[1];
struct vslf *vslf;
struct vbitmap *tags = NULL;
+ char errbuf[VRE_ERROR_LEN];
+
CHECK_OBJ_NOTNULL(vsl, VSL_MAGIC);
AN(arg);
@@ -283,8 +287,12 @@ vsl_IX_arg(struct VSL_data *vsl, int opt, const char *arg)
if (vre == NULL) {
if (tags)
vbit_destroy(tags);
+ AN(VSB_init(vsb, errbuf, sizeof errbuf));
+ AZ(VRE_error(vsb, err));
+ AZ(VSB_finish(vsb));
+ VSB_fini(vsb);
return (vsl_diag(vsl, "-%c: Regex error at position %d (%s)",
- (char)opt, off, err));
+ (char)opt, off, errbuf));
}
ALLOC_OBJ(vslf, VSLF_MAGIC);
diff --git a/lib/libvarnishapi/vxp_parse.c b/lib/libvarnishapi/vxp_parse.c
index 1dfd42065..65ebff9a8 100644
--- a/lib/libvarnishapi/vxp_parse.c
+++ b/lib/libvarnishapi/vxp_parse.c
@@ -260,8 +260,7 @@ vxp_expr_str(struct vxp *vxp, struct vex_rhs **prhs)
static void
vxp_expr_regex(struct vxp *vxp, struct vex_rhs **prhs)
{
- const char *errptr;
- int erroff;
+ int err, erroff;
/* XXX: Caseless option */
@@ -279,10 +278,11 @@ vxp_expr_regex(struct vxp *vxp, struct vex_rhs **prhs)
(*prhs)->type = VEX_REGEX;
(*prhs)->val_string = strdup(vxp->t->dec);
(*prhs)->val_regex = VRE_compile(vxp->t->dec, vxp->vre_options,
- &errptr, &erroff);
+ &err, &erroff);
if ((*prhs)->val_regex == NULL) {
- AN(errptr);
- VSB_printf(vxp->sb, "Regular expression error: %s ", errptr);
+ VSB_cat(vxp->sb, "Regular expression error: ");
+ AZ(VRE_error(vxp->sb, err));
+ VSB_putc(vxp->sb, ' ');
vxp_ErrWhere(vxp, vxp->t, erroff);
return;
}
diff --git a/lib/libvcc/vcc_utils.c b/lib/libvcc/vcc_utils.c
index de064bea0..bc39936ff 100644
--- a/lib/libvcc/vcc_utils.c
+++ b/lib/libvcc/vcc_utils.c
@@ -53,16 +53,16 @@ vcc_regexp(struct vcc *tl, struct vsb *vgc_name)
{
char buf[BUFSIZ];
vre_t *t;
- const char *error;
- int erroroffset;
+ int error, erroroffset;
struct inifin *ifp;
assert(tl->t->tok == CSTR);
t = VRE_compile(tl->t->dec, 0, &error, &erroroffset);
if (t == NULL) {
- VSB_printf(tl->sb,
- "Regexp compilation error:\n\n%s\n\n", error);
+ VSB_cat(tl->sb, "Regexp compilation error:\n\n");
+ AZ(VRE_error(tl->sb, error));
+ VSB_cat(tl->sb, "\n\n");
vcc_ErrWhere(tl, tl->t);
return;
}
More information about the varnish-commit
mailing list