From: Phil Sutter <phil@nwl.cc>
To: Florian Westphal <fw@strlen.de>
Cc: netfilter-devel <netfilter-devel@vger.kernel.org>
Subject: Re: [PATCH v3 iptables-nft 1/3] xlate: get rid of escape_quotes
Date: Wed, 30 Nov 2022 13:35:08 +0100 [thread overview]
Message-ID: <Y4dN/NLxE2miZaFZ@orbyte.nwl.cc> (raw)
In-Reply-To: <20221130093154.29004-2-fw@strlen.de>
On Wed, Nov 30, 2022 at 10:31:52AM +0100, Florian Westphal wrote:
> Its not necessary to escape " characters, we can let xtables-translate
> print the entire translation/command enclosed in '' chracters, i.e. nft
> 'add rule ...', this also takes care of [, { and other special characters
> that some shells might parse otherwise (when copy-pasting translated output).
>
> The escape_quotes struct member is retained to avoid an ABI breakage.
>
> This breaks all xlate test cases, fixup in followup patches.
>
> v3: no need to escape ', replace strcmp(x, "") with x[0] (Phil Sutter)
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> extensions/libebt_log.c | 8 ++------
> extensions/libebt_nflog.c | 8 ++------
> extensions/libxt_LOG.c | 10 +++-------
> extensions/libxt_NFLOG.c | 12 ++++--------
> extensions/libxt_comment.c | 7 +------
> extensions/libxt_helper.c | 8 ++------
> include/xtables.h | 4 ++--
> iptables/nft-bridge.c | 2 --
> iptables/xtables-eb-translate.c | 12 ++++++------
> iptables/xtables-translate.c | 22 ++++++++++------------
> 10 files changed, 32 insertions(+), 61 deletions(-)
>
> diff --git a/extensions/libebt_log.c b/extensions/libebt_log.c
> index 13c7fafecb11..045062196d20 100644
> --- a/extensions/libebt_log.c
> +++ b/extensions/libebt_log.c
> @@ -181,12 +181,8 @@ static int brlog_xlate(struct xt_xlate *xl,
> const struct ebt_log_info *loginfo = (const void *)params->target->data;
>
> xt_xlate_add(xl, "log");
> - if (loginfo->prefix[0]) {
> - if (params->escape_quotes)
> - xt_xlate_add(xl, " prefix \\\"%s\\\"", loginfo->prefix);
> - else
> - xt_xlate_add(xl, " prefix \"%s\"", loginfo->prefix);
> - }
> + if (loginfo->prefix[0])
> + xt_xlate_add(xl, " prefix \"%s\"", loginfo->prefix);
>
> if (loginfo->loglevel != LOG_DEFAULT_LEVEL)
> xt_xlate_add(xl, " level %s", eight_priority[loginfo->loglevel].c_name);
> diff --git a/extensions/libebt_nflog.c b/extensions/libebt_nflog.c
> index 9801f358c81b..115e15da4584 100644
> --- a/extensions/libebt_nflog.c
> +++ b/extensions/libebt_nflog.c
> @@ -130,12 +130,8 @@ static int brnflog_xlate(struct xt_xlate *xl,
> const struct ebt_nflog_info *info = (void *)params->target->data;
>
> xt_xlate_add(xl, "log ");
> - if (info->prefix[0] != '\0') {
> - if (params->escape_quotes)
> - xt_xlate_add(xl, "prefix \\\"%s\\\" ", info->prefix);
> - else
> - xt_xlate_add(xl, "prefix \"%s\" ", info->prefix);
> - }
> + if (info->prefix[0] != '\0')
> + xt_xlate_add(xl, "prefix \"%s\" ", info->prefix);
>
> xt_xlate_add(xl, "group %u ", info->group);
>
> diff --git a/extensions/libxt_LOG.c b/extensions/libxt_LOG.c
> index e3f4290ba003..cfde0c7bca6a 100644
> --- a/extensions/libxt_LOG.c
> +++ b/extensions/libxt_LOG.c
> @@ -116,7 +116,7 @@ static void LOG_print(const void *ip, const struct xt_entry_target *target,
> printf(" unknown-flags");
> }
>
> - if (strcmp(loginfo->prefix, "") != 0)
> + if (loginfo->prefix[0] != 0)
> printf(" prefix \"%s\"", loginfo->prefix);
> }
>
Wrong spot? Because:
> @@ -151,12 +151,8 @@ static int LOG_xlate(struct xt_xlate *xl,
> const char *pname = priority2name(loginfo->level);
>
> xt_xlate_add(xl, "log");
> - if (strcmp(loginfo->prefix, "") != 0) {
> - if (params->escape_quotes)
> - xt_xlate_add(xl, " prefix \\\"%s\\\"", loginfo->prefix);
> - else
> - xt_xlate_add(xl, " prefix \"%s\"", loginfo->prefix);
> - }
> + if (strcmp(loginfo->prefix, "") != 0)
> + xt_xlate_add(xl, " prefix \"%s\"", loginfo->prefix);
>
> if (loginfo->level != LOG_DEFAULT_LEVEL && pname)
> xt_xlate_add(xl, " level %s", pname);
Here's still strcmp(). Since it doesn't make a difference in the binary
though, I'm fine with leaving the strcmp() calls as-is.
[...]
> diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
> index 4e8db4bedff8..1f16e726d3a7 100644
> --- a/iptables/xtables-translate.c
> +++ b/iptables/xtables-translate.c
[...]
> @@ -150,6 +148,7 @@ static int nft_rule_xlate_add(struct nft_handle *h,
> bool append)
> {
> struct xt_xlate *xl = xt_xlate_alloc(10240);
> + const char *tick = cs->restore ? "" : "\'";
Left-over tick escaping here.
Thanks, Phil
next prev parent reply other threads:[~2022-11-30 12:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-30 9:31 [PATCH v3 iptables-nft 0/3] remove escape_quotes support Florian Westphal
2022-11-30 9:31 ` [PATCH v3 iptables-nft 1/3] xlate: get rid of escape_quotes Florian Westphal
2022-11-30 12:35 ` Phil Sutter [this message]
2022-11-30 9:31 ` [PATCH v3 iptables-nft 2/3] extensions: change expected output for new format Florian Westphal
2022-11-30 9:31 ` [PATCH v3 iptables-nft 3/3] xlate-test: avoid shell entanglements Florian Westphal
2022-11-30 19:37 ` [PATCH v3 iptables-nft 0/3] remove escape_quotes support Phil Sutter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y4dN/NLxE2miZaFZ@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=fw@strlen.de \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).