Re: Make filter line handling more developer friendly
On Mon, Aug 26, 2019 at 12:04:06PM +0200, Martijn van Duren wrote:
> On 8/26/19 11:42 AM, Sebastien Marie wrote:
> > On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
> >> Right now all we get is "Misbehaving filter", which doesn't tell the
> >> developer a lot.
> >>
> >> Diff below does the following:
> >> - Make the register_hooks actually fail on misbehaviour.
> >> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> >> - Hoist some checks from lka_filter_process_response to processor_io
> >> - Make lka_filter_process_response void (it fatals now)
> >> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> >> - change str{,n}casecmp to str{,n}cmp for consistency.
> >> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> >> - restructure lka_filter_process_response to be shorter.
> >> and most importantly
> >> - Add a descriptive fatal() minefield.
> >>
> >> -12 LoC.
> >>
> >> Tested with filter-dnsbl with both a successful and rejected connection.
> >>
> >> OK?
> >
> > just one remark.
> >
> >> Index: lka_filter.c
> >> ===
> >> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> >> retrieving revision 1.41
> >> diff -u -p -r1.41 lka_filter.c
> >> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> >> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> >> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> >>struct filter_session *fs;
> >>
> >>(void)strlcpy(buffer, line, sizeof buffer);
> >> - if ((ep = strchr(buffer, '|')) == NULL)
> >> - return 0;
> >> - *ep = 0;
> >> + ep = strchr(buffer, '|');
> >> + ep[0] = '\0';
> >
> > is it possible to buffer to not have '|' ? if yes, you could deference NULL.
> >
> >
> You're absolutely right.
> It's not a risk, since both would crash smtpd, but doing the check would
> result in a clearer error message, which is the purpose of this
> endeavour.
>
ok gilles@
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 26 Aug 2019 10:03:43 -
> @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> static void filter_result_disconnect(uint64_t, const char *);
>
> static void filter_session_io(struct io *, int, void *);
> -int lka_filter_process_response(const char *, const char *);
> +void lka_filter_process_response(const char *, const char *);
>
>
> struct filter_session {
> @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> hook += 8;
> }
> else
> - return;
> + fatalx("Invalid message direction: %s", hook);
>
> for (i = 0; i < nitems(filter_execs); i++)
> if (strcmp(hook, filter_execs[i].phase_name) == 0)
> break;
> if (i == nitems(filter_execs))
> - return;
> + fatalx("Unrecognized report name: %s", hook);
>
> iter = NULL;
> while (dict_iter(, , _name, (void **)))
> @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> }
> }
>
> -int
> +void
> lka_filter_process_response(const char *name, const char *line)
> {
> uint64_t reqid;
> @@ -430,87 +430,68 @@ lka_filter_process_response(const char *
>
> (void)strlcpy(buffer, line, sizeof buffer);
> if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing token: %s", line);
> + ep[0] = '\0';
>
> kind = buffer;
> - if (strcmp(kind, "register") == 0)
> - return 1;
> -
> - if (strcmp(kind, "filter-result") != 0 &&
> - strcmp(kind, "filter-dataline") != 0)
> - return 0;
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing reqid: %s", line);
> + ep[0] = '\0';
>
> token = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && token == ULONG_MAX)
> - return 0;
> + fatalx("Invalid token: %s", line);
> + if (errno == ERANGE && token == ULLONG_MAX)
> + fatal("Invalid token: %s", line);
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatal("Missing directive: %s", line);
> + ep[0] = '\0';
>
> reqid = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && reqid == ULONG_MAX)
> - return 0;
> + fatalx("Invalid reqid: %s", line);
> + if (errno == ERANGE && reqid == ULLONG_MAX)
> + fatal("Invalid reqid: %s", line);
>
Re: Make filter line handling more developer friendly
On 8/26/19 11:42 AM, Sebastien Marie wrote:
> On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
>> Right now all we get is "Misbehaving filter", which doesn't tell the
>> developer a lot.
>>
>> Diff below does the following:
>> - Make the register_hooks actually fail on misbehaviour.
>> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
>> - Hoist some checks from lka_filter_process_response to processor_io
>> - Make lka_filter_process_response void (it fatals now)
>> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
>> - change str{,n}casecmp to str{,n}cmp for consistency.
>> - change an fugly "nextline:" "goto nextline" loop to an actual while.
>> - restructure lka_filter_process_response to be shorter.
>> and most importantly
>> - Add a descriptive fatal() minefield.
>>
>> -12 LoC.
>>
>> Tested with filter-dnsbl with both a successful and rejected connection.
>>
>> OK?
>
> just one remark.
>
>> Index: lka_filter.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
>> retrieving revision 1.41
>> diff -u -p -r1.41 lka_filter.c
>> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
>> +++ lka_filter.c 25 Aug 2019 04:47:47 -
>> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
>> struct filter_session *fs;
>>
>> (void)strlcpy(buffer, line, sizeof buffer);
>> -if ((ep = strchr(buffer, '|')) == NULL)
>> -return 0;
>> -*ep = 0;
>> +ep = strchr(buffer, '|');
>> +ep[0] = '\0';
>
> is it possible to buffer to not have '|' ? if yes, you could deference NULL.
>
>
You're absolutely right.
It's not a risk, since both would crash smtpd, but doing the check would
result in a clearer error message, which is the purpose of this
endeavour.
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.41
diff -u -p -r1.41 lka_filter.c
--- lka_filter.c18 Aug 2019 16:52:02 - 1.41
+++ lka_filter.c26 Aug 2019 10:03:43 -
@@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
static voidfilter_result_disconnect(uint64_t, const char *);
static voidfilter_session_io(struct io *, int, void *);
-intlka_filter_process_response(const char *, const char *);
+void lka_filter_process_response(const char *, const char *);
struct filter_session {
@@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
hook += 8;
}
else
- return;
+ fatalx("Invalid message direction: %s", hook);
for (i = 0; i < nitems(filter_execs); i++)
if (strcmp(hook, filter_execs[i].phase_name) == 0)
break;
if (i == nitems(filter_execs))
- return;
+ fatalx("Unrecognized report name: %s", hook);
iter = NULL;
while (dict_iter(, , _name, (void **)))
@@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
}
}
-int
+void
lka_filter_process_response(const char *name, const char *line)
{
uint64_t reqid;
@@ -430,87 +430,68 @@ lka_filter_process_response(const char *
(void)strlcpy(buffer, line, sizeof buffer);
if ((ep = strchr(buffer, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatalx("Missing token: %s", line);
+ ep[0] = '\0';
kind = buffer;
- if (strcmp(kind, "register") == 0)
- return 1;
-
- if (strcmp(kind, "filter-result") != 0 &&
- strcmp(kind, "filter-dataline") != 0)
- return 0;
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatalx("Missing reqid: %s", line);
+ ep[0] = '\0';
token = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && token == ULONG_MAX)
- return 0;
+ fatalx("Invalid token: %s", line);
+ if (errno == ERANGE && token == ULLONG_MAX)
+ fatal("Invalid token: %s", line);
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatal("Missing directive: %s", line);
+ ep[0] = '\0';
reqid = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && reqid == ULONG_MAX)
- return 0;
+ fatalx("Invalid reqid: %s", line);
+ if (errno == ERANGE && reqid == ULLONG_MAX)
+ fatal("Invalid reqid: %s", line);
response = ep+1;
fs = tree_xget(, reqid);
if (strcmp(kind, "filter-dataline") == 0) {
if (fs->phase != FILTER_DATA_LINE)
- fatalx("misbehaving filter");
Re: Make filter line handling more developer friendly
On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
> Right now all we get is "Misbehaving filter", which doesn't tell the
> developer a lot.
>
> Diff below does the following:
> - Make the register_hooks actually fail on misbehaviour.
> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> - Hoist some checks from lka_filter_process_response to processor_io
> - Make lka_filter_process_response void (it fatals now)
> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> - change str{,n}casecmp to str{,n}cmp for consistency.
> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> - restructure lka_filter_process_response to be shorter.
> and most importantly
> - Add a descriptive fatal() minefield.
>
> -12 LoC.
>
> Tested with filter-dnsbl with both a successful and rejected connection.
>
> OK?
just one remark.
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> struct filter_session *fs;
>
> (void)strlcpy(buffer, line, sizeof buffer);
> - if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + ep = strchr(buffer, '|');
> + ep[0] = '\0';
is it possible to buffer to not have '|' ? if yes, you could deference NULL.
--
Sebastien Marie
Re: Make filter line handling more developer friendly
On Mon, Aug 26, 2019, at 00:30, gil...@poolp.org wrote:
> 25 août 2019 07:01 "Martijn van Duren" a
> écrit:
> > Right now all we get is "Misbehaving filter", which doesn't tell the
> > developer a lot.
> >
>
> Indeed
>
>
> > Diff below does the following:
> > - Make the register_hooks actually fail on misbehaviour.
> > - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> > - Hoist some checks from lka_filter_process_response to processor_io
> > - Make lka_filter_process_response void (it fatals now)
> > - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> > - change str{,n}casecmp to str{,n}cmp for consistency.
> > - change an fugly "nextline:" "goto nextline" loop to an actual while.
> > - restructure lka_filter_process_response to be shorter.
> > and most importantly
> > - Add a descriptive fatal() minefield.
> >
> > -12 LoC.
> >
> > Tested with filter-dnsbl with both a successful and rejected connection.
> >
> > OK?
> >
> > martijn@
> >
>
> Reads fine but I'll give it a better read today and run with it on my
> machines for a day see if I observe any issues with filter-rspamd and
> filter-senderscore as well as some builtin filters.
>
>
> > Index: lka_filter.c
> > ===
> > RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 lka_filter.c
> > --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> > +++ lka_filter.c 25 Aug 2019 04:47:47 -
> > @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> > static void filter_result_disconnect(uint64_t, const char *);
> >
> > static void filter_session_io(struct io *, int, void *);
> > -int lka_filter_process_response(const char *, const char *);
> > +void lka_filter_process_response(const char *, const char *);
> >
> > struct filter_session {
> > @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> > hook += 8;
> > }
> > else
> > - return;
> > + fatalx("Invalid message direction: %s", hook);
> >
> > for (i = 0; i < nitems(filter_execs); i++)
> > if (strcmp(hook, filter_execs[i].phase_name) == 0)
> > break;
> > if (i == nitems(filter_execs))
> > - return;
> > + fatalx("Unrecognized report name: %s", hook);
> >
> > iter = NULL;
> > while (dict_iter(, , _name, (void **)))
> > @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> > }
> > }
> >
> > -int
> > +void
> > lka_filter_process_response(const char *name, const char *line)
> > {
> > uint64_t reqid;
> > @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> > struct filter_session *fs;
> >
> > (void)strlcpy(buffer, line, sizeof buffer);
> > - if ((ep = strchr(buffer, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + ep = strchr(buffer, '|');
> > + ep[0] = '\0';
> >
> > kind = buffer;
> > - if (strcmp(kind, "register") == 0)
> > - return 1;
> > -
> > - if (strcmp(kind, "filter-result") != 0 &&
> > - strcmp(kind, "filter-dataline") != 0)
> > - return 0;
> >
> > qid = ep+1;
> > if ((ep = strchr(qid, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + fatalx("Missing reqid: %s", line);
> > + ep[0] = '\0';
> >
> > token = strtoull(qid, , 16);
> > if (qid[0] == '\0' || *ep != '\0')
> > - return 0;
> > - if (errno == ERANGE && token == ULONG_MAX)
> > - return 0;
> > + fatalx("Invalid token: %s", line);
> > + if (errno == ERANGE && token == ULLONG_MAX)
> > + fatal("Invalid token: %s", line);
> >
> > qid = ep+1;
> > if ((ep = strchr(qid, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + fatal("Missing directive: %s", line);
> > + ep[0] = '\0';
> >
> > reqid = strtoull(qid, , 16);
> > if (qid[0] == '\0' || *ep != '\0')
> > - return 0;
> > - if (errno == ERANGE && reqid == ULONG_MAX)
> > - return 0;
> > + fatalx("Invalid reqid: %s", line);
> > + if (errno == ERANGE && reqid == ULLONG_MAX)
> > + fatal("Invalid reqid: %s", line);
> >
> > response = ep+1;
> >
> > fs = tree_xget(, reqid);
> > if (strcmp(kind, "filter-dataline") == 0) {
> > if (fs->phase != FILTER_DATA_LINE)
> > - fatalx("misbehaving filter");
> > + fatalx("filter-dataline out of dataline phase");
> > filter_data_next(token, reqid, response);
> > - return 1;
> > + return;
> > }
> > if (fs->phase == FILTER_DATA_LINE)
> > - fatalx("misbehaving filter");
> > + fatalx("filter-result in dataline phase");
> >
> > if ((ep = strchr(response, '|'))) {
> > parameter = ep + 1;
> > - *ep = 0;
> > + ep[0] = '\0';
> > }
> >
> > - if (strcmp(response, "proceed") != 0 &&
> > - strcmp(response, "reject") != 0 &&
> > - strcmp(response, "disconnect") != 0 &&
> > - strcmp(response, "rewrite") != 0)
> > - return 0;
> > -
> > - if (strcmp(response, "proceed") == 0 &&
> > - parameter)
> > - return 0;
> > -
> > - if (strcmp(response, "proceed") != 0 &&
> > - parameter == NULL)
> > - return 0;
> > -
> > - if (strcmp(response, "rewrite") == 0) {
> > - filter_result_rewrite(reqid, parameter);
> > - return 1;
> > - }
> > -
> > - if (strcmp(response, "reject") == 0) {
> > -
Re: Make filter line handling more developer friendly
25 août 2019 07:01 "Martijn van Duren" a
écrit:
> Right now all we get is "Misbehaving filter", which doesn't tell the
> developer a lot.
>
Indeed
> Diff below does the following:
> - Make the register_hooks actually fail on misbehaviour.
> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> - Hoist some checks from lka_filter_process_response to processor_io
> - Make lka_filter_process_response void (it fatals now)
> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> - change str{,n}casecmp to str{,n}cmp for consistency.
> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> - restructure lka_filter_process_response to be shorter.
> and most importantly
> - Add a descriptive fatal() minefield.
>
> -12 LoC.
>
> Tested with filter-dnsbl with both a successful and rejected connection.
>
> OK?
>
> martijn@
>
Reads fine but I'll give it a better read today and run with it on my
machines for a day see if I observe any issues with filter-rspamd and
filter-senderscore as well as some builtin filters.
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> static void filter_result_disconnect(uint64_t, const char *);
>
> static void filter_session_io(struct io *, int, void *);
> -int lka_filter_process_response(const char *, const char *);
> +void lka_filter_process_response(const char *, const char *);
>
> struct filter_session {
> @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> hook += 8;
> }
> else
> - return;
> + fatalx("Invalid message direction: %s", hook);
>
> for (i = 0; i < nitems(filter_execs); i++)
> if (strcmp(hook, filter_execs[i].phase_name) == 0)
> break;
> if (i == nitems(filter_execs))
> - return;
> + fatalx("Unrecognized report name: %s", hook);
>
> iter = NULL;
> while (dict_iter(, , _name, (void **)))
> @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> }
> }
>
> -int
> +void
> lka_filter_process_response(const char *name, const char *line)
> {
> uint64_t reqid;
> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> struct filter_session *fs;
>
> (void)strlcpy(buffer, line, sizeof buffer);
> - if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + ep = strchr(buffer, '|');
> + ep[0] = '\0';
>
> kind = buffer;
> - if (strcmp(kind, "register") == 0)
> - return 1;
> -
> - if (strcmp(kind, "filter-result") != 0 &&
> - strcmp(kind, "filter-dataline") != 0)
> - return 0;
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing reqid: %s", line);
> + ep[0] = '\0';
>
> token = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && token == ULONG_MAX)
> - return 0;
> + fatalx("Invalid token: %s", line);
> + if (errno == ERANGE && token == ULLONG_MAX)
> + fatal("Invalid token: %s", line);
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatal("Missing directive: %s", line);
> + ep[0] = '\0';
>
> reqid = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && reqid == ULONG_MAX)
> - return 0;
> + fatalx("Invalid reqid: %s", line);
> + if (errno == ERANGE && reqid == ULLONG_MAX)
> + fatal("Invalid reqid: %s", line);
>
> response = ep+1;
>
> fs = tree_xget(, reqid);
> if (strcmp(kind, "filter-dataline") == 0) {
> if (fs->phase != FILTER_DATA_LINE)
> - fatalx("misbehaving filter");
> + fatalx("filter-dataline out of dataline phase");
> filter_data_next(token, reqid, response);
> - return 1;
> + return;
> }
> if (fs->phase == FILTER_DATA_LINE)
> - fatalx("misbehaving filter");
> + fatalx("filter-result in dataline phase");
>
> if ((ep = strchr(response, '|'))) {
> parameter = ep + 1;
> - *ep = 0;
> + ep[0] = '\0';
> }
>
> - if (strcmp(response, "proceed") != 0 &&
> - strcmp(response, "reject") != 0 &&
> - strcmp(response, "disconnect") != 0 &&
> - strcmp(response, "rewrite") != 0)
> - return 0;
> -
> - if (strcmp(response, "proceed") == 0 &&
> - parameter)
> - return 0;
> -
> - if (strcmp(response, "proceed") != 0 &&
> - parameter == NULL)
> - return 0;
> -
> - if (strcmp(response, "rewrite") == 0) {
> - filter_result_rewrite(reqid, parameter);
> - return 1;
> - }
> -
> - if (strcmp(response, "reject") == 0) {
> - filter_result_reject(reqid, parameter);
> - return 1;
> - }
> -
> - if (strcmp(response, "disconnect") == 0) {
> - filter_result_disconnect(reqid, parameter);
> - return 1;
> + if (strcmp(response, "proceed") == 0) {
> + if (parameter != NULL)
> + fatalx("Unexpected parameter after proceed: %s", line);
> + filter_protocol_next(token, reqid, 0);
> + return;
> + } else {
> + if (parameter == NULL)
> +
Make filter line handling more developer friendly
Right now all we get is "Misbehaving filter", which doesn't tell the
developer a lot.
Diff below does the following:
- Make the register_hooks actually fail on misbehaviour.
- Change some *ep = 0 to a more semantically correct ep[0] = '\0'
- Hoist some checks from lka_filter_process_response to processor_io
- Make lka_filter_process_response void (it fatals now)
- strtoull returns ULLONG_MAX on error, not ULONG_MAX
- change str{,n}casecmp to str{,n}cmp for consistency.
- change an fugly "nextline:" "goto nextline" loop to an actual while.
- restructure lka_filter_process_response to be shorter.
and most importantly
- Add a descriptive fatal() minefield.
-12 LoC.
Tested with filter-dnsbl with both a successful and rejected connection.
OK?
martijn@
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.41
diff -u -p -r1.41 lka_filter.c
--- lka_filter.c18 Aug 2019 16:52:02 - 1.41
+++ lka_filter.c25 Aug 2019 04:47:47 -
@@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
static voidfilter_result_disconnect(uint64_t, const char *);
static voidfilter_session_io(struct io *, int, void *);
-intlka_filter_process_response(const char *, const char *);
+void lka_filter_process_response(const char *, const char *);
struct filter_session {
@@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
hook += 8;
}
else
- return;
+ fatalx("Invalid message direction: %s", hook);
for (i = 0; i < nitems(filter_execs); i++)
if (strcmp(hook, filter_execs[i].phase_name) == 0)
break;
if (i == nitems(filter_execs))
- return;
+ fatalx("Unrecognized report name: %s", hook);
iter = NULL;
while (dict_iter(, , _name, (void **)))
@@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
}
}
-int
+void
lka_filter_process_response(const char *name, const char *line)
{
uint64_t reqid;
@@ -429,88 +429,68 @@ lka_filter_process_response(const char *
struct filter_session *fs;
(void)strlcpy(buffer, line, sizeof buffer);
- if ((ep = strchr(buffer, '|')) == NULL)
- return 0;
- *ep = 0;
+ ep = strchr(buffer, '|');
+ ep[0] = '\0';
kind = buffer;
- if (strcmp(kind, "register") == 0)
- return 1;
-
- if (strcmp(kind, "filter-result") != 0 &&
- strcmp(kind, "filter-dataline") != 0)
- return 0;
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatalx("Missing reqid: %s", line);
+ ep[0] = '\0';
token = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && token == ULONG_MAX)
- return 0;
+ fatalx("Invalid token: %s", line);
+ if (errno == ERANGE && token == ULLONG_MAX)
+ fatal("Invalid token: %s", line);
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatal("Missing directive: %s", line);
+ ep[0] = '\0';
reqid = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && reqid == ULONG_MAX)
- return 0;
+ fatalx("Invalid reqid: %s", line);
+ if (errno == ERANGE && reqid == ULLONG_MAX)
+ fatal("Invalid reqid: %s", line);
response = ep+1;
fs = tree_xget(, reqid);
if (strcmp(kind, "filter-dataline") == 0) {
if (fs->phase != FILTER_DATA_LINE)
- fatalx("misbehaving filter");
+ fatalx("filter-dataline out of dataline phase");
filter_data_next(token, reqid, response);
- return 1;
+ return;
}
if (fs->phase == FILTER_DATA_LINE)
- fatalx("misbehaving filter");
+ fatalx("filter-result in dataline phase");
if ((ep = strchr(response, '|'))) {
parameter = ep + 1;
- *ep = 0;
+ ep[0] = '\0';
}
- if (strcmp(response, "proceed") != 0 &&
- strcmp(response, "reject") != 0 &&
- strcmp(response, "disconnect") != 0 &&
- strcmp(response, "rewrite") != 0)
- return 0;
-
- if (strcmp(response, "proceed") == 0 &&
- parameter)
- return 0;
-
- if (strcmp(response, "proceed") != 0 &&
- parameter == NULL)
- return 0;
-
- if (strcmp(response, "rewrite") == 0) {
- filter_result_rewrite(reqid, parameter);
- return 1;
- }
