| Message ID | 20171123185514.16711-1-harshasharmaiitr@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | evaluate: print error for null string befort assert statement | expand |
Harsha Sharma <harshasharmaiitr@gmail.com> wrote: > Print error "Null string is not allowed" before assert statement. > For e.g. > nft add rule filter input meta iifname '""' > Error: Null String is not allowed > add rule filter input meta iifname "" Is there any case where "" should be allowed? If not, I'd rather change scanner.l to not recognize "" as a quoted string. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Nov 23, 2017 at 10:25:51PM +0100, Florian Westphal wrote: > Harsha Sharma <harshasharmaiitr@gmail.com> wrote: > > Print error "Null string is not allowed" before assert statement. > > For e.g. > > nft add rule filter input meta iifname '""' > > Error: Null String is not allowed > > add rule filter input meta iifname "" > > Is there any case where "" should be allowed? > > If not, I'd rather change scanner.l to not recognize "" as > a quoted string. We'll get error reporting like this if we handle this from the scanner: # nft add rule x y ct label \"\" Error: syntax error, unexpected junk add rule x y ct label "" ^ This is just two extra lines in the validation step and it is not worth to optimize an error case, ie. do it earlier. So I'm feeling inclined to take this one. I will mangle the error message to "Empty string is not allowed". "Null" probably sounds too programmer thing. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Nov 23, 2017 at 10:25:51PM +0100, Florian Westphal wrote: > > Harsha Sharma <harshasharmaiitr@gmail.com> wrote: > > > Print error "Null string is not allowed" before assert statement. > > > For e.g. > > > nft add rule filter input meta iifname '""' > > > Error: Null String is not allowed > > > add rule filter input meta iifname "" > > > > Is there any case where "" should be allowed? > > > > If not, I'd rather change scanner.l to not recognize "" as > > a quoted string. > > We'll get error reporting like this if we handle this from the > scanner: > > # nft add rule x y ct label \"\" > Error: syntax error, unexpected junk > add rule x y ct label "" Right, we'd have to add dummy empty_string rule to avoid this. > This is just two extra lines in the validation step and it is not > worth to optimize an error case, ie. do it earlier. So I'm feeling > inclined to take this one. Makes sense to me. > I will mangle the error message to "Empty string is not allowed". > "Null" probably sounds too programmer thing. Can you also get rid of the assert(strlen ... line? Its useless after this patch. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 27, 2017 at 11:48:03PM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Thu, Nov 23, 2017 at 10:25:51PM +0100, Florian Westphal wrote: > > > Harsha Sharma <harshasharmaiitr@gmail.com> wrote: > > > > Print error "Null string is not allowed" before assert statement. > > > > For e.g. > > > > nft add rule filter input meta iifname '""' > > > > Error: Null String is not allowed > > > > add rule filter input meta iifname "" > > > > > > Is there any case where "" should be allowed? > > > > > > If not, I'd rather change scanner.l to not recognize "" as > > > a quoted string. > > > > We'll get error reporting like this if we handle this from the > > scanner: > > > > # nft add rule x y ct label \"\" > > Error: syntax error, unexpected junk > > add rule x y ct label "" > > Right, we'd have to add dummy empty_string rule to avoid this. > > > This is just two extra lines in the validation step and it is not > > worth to optimize an error case, ie. do it earlier. So I'm feeling > > inclined to take this one. > > Makes sense to me. > > > I will mangle the error message to "Empty string is not allowed". > > "Null" probably sounds too programmer thing. > > Can you also get rid of the assert(strlen ... line? > Its useless after this patch. Indeed, done! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/evaluate.c b/src/evaluate.c index fd61e75..ad044a4 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -235,6 +235,10 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp) memset(data + len, 0, data_len - len); mpz_export_data(data, expr->value, BYTEORDER_HOST_ENDIAN, len); + if (strlen(data) == 0) { + return expr_error(ctx->msgs, expr, + "Null String is not allowed"); + } assert(strlen(data) > 0); datalen = strlen(data) - 1; if (data[datalen] != '*') {
Print error "Null string is not allowed" before assert statement. For e.g. nft add rule filter input meta iifname '""' Error: Null String is not allowed add rule filter input meta iifname "" Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com> --- src/evaluate.c | 4 ++++ 1 file changed, 4 insertions(+)