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?
[...]
> diff --git a/iptables/xtables-eb-translate.c b/iptables/xtables-eb-translate.c
> index f09883cd518c..0cf215b9c6b6 100644
> --- a/iptables/xtables-eb-translate.c
> +++ b/iptables/xtables-eb-translate.c
> @@ -159,15 +159,16 @@ static int nft_rule_eb_xlate_add(struct nft_handle *h, const struct xt_cmd_parse
> int ret;
>
> if (append) {
> - xt_xlate_add(xl, "add rule bridge %s %s ", p->table, p->chain);
> + xt_xlate_add(xl, "'add rule bridge %s %s ", p->table, p->chain);
> } else {
> - xt_xlate_add(xl, "insert rule bridge %s %s ", p->table, p->chain);
> + xt_xlate_add(xl, "'insert rule bridge %s %s ", p->table, p->chain);
> }
>
> ret = h->ops->xlate(cs, xl);
> 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?
> diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
> index d1e87f167df7..0589ac229746 100644
> --- a/iptables/xtables-translate.c
> +++ b/iptables/xtables-translate.c
[...]
> @@ -165,13 +163,16 @@ static int nft_rule_xlate_add(struct nft_handle *h,
>
> set = xt_xlate_set_get(xl);
> 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.
> if (!cs->restore && p->command != CMD_NONE)
> printf("nft ");
> }
>
> + if (!cs->restore)
> + printf("%c", '\'');
> +
> if (append) {
> printf("add rule %s %s %s ",
> family2str[h->family], p->table, p->chain);
> @@ -179,7 +180,12 @@ static int nft_rule_xlate_add(struct nft_handle *h,
> printf("insert rule %s %s %s ",
> family2str[h->family], p->table, p->chain);
> }
> - printf("%s\n", xt_xlate_rule_get(xl));
> + printf("%s", xt_xlate_rule_get(xl));
> +
> + if (!cs->restore)
> + printf("%c", '\'');
> +
> + puts("");
>
> err_out:
> xt_xlate_free(xl);
Cheers, Phil
