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@xxxxxxxxx>
> ---
> 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
