Phil Sutter <phil@xxxxxx> wrote:
> On Thu, Nov 24, 2022 at 02:49:37PM +0100, Florian Westphal wrote:
> > Its not necessary to escape " characters, we can simply
> > 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).
> >
> > This breaks all xlate test cases, fixup in followup patches.
> >
> > Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> > ---
> [...]
> > diff --git a/include/xtables.h b/include/xtables.h
> > index 9eba4f619d35..150d40bfafd9 100644
> > --- a/include/xtables.h
> > +++ b/include/xtables.h
> > @@ -211,14 +211,12 @@ struct xt_xlate_mt_params {
> > const void *ip;
> > const struct xt_entry_match *match;
> > int numeric;
> > - bool escape_quotes;
> > };
> >
> > struct xt_xlate_tg_params {
> > const void *ip;
> > const struct xt_entry_target *target;
> > int numeric;
> > - bool escape_quotes;
> > };
>
> Does this break ABI compatibility?
Yes. I can keep the bool as a dead member if you prefer.
> > if (ret)
> > - printf("%s\n", xt_xlate_get(xl));
> > + printf("%s", xt_xlate_get(xl));
> >
> > + puts("'");
> > xt_xlate_free(xl);
> > return ret;
> > }
>
> If h->ops->xlate() fails, the code prints "'\n". How about:
>
> | if (ret)
> | printf("%s'\n", xt_xlate_get(xl));
>
> Or am I missing something?
We already printed 'insert rule, hence it was weird for the '\n' to be missed, but I see that
the caller will print a ' # iptables-syntax' in that case, so I will
re-add the '\n' to where it was.
> > if (set[0]) {
> > - printf("add set %s %s %s\n", family2str[h->family], p->table,
> > + printf("'add set %s %s %s'\n", family2str[h->family], p->table,
> > xt_xlate_set_get(xl));
>
> Quoting needs to respect cs->restore value, no? Maybe simpler to
> introduce 'const char *tick = cs->restore ? "" : "'";' and just insert
> it everywhere needed.
Will do that.
