Re: [PATCH v6 3/4] net/slirp.c: Refactor address parsing

From: Doug Evans <dje@google.com>
To: QEMU Developers <qemu-devel@nongnu.org>
Cc: "Samuel Thibault" <samuel.thibault@ens-lyon.org>,
	"Daniel P . Berrangé" <berrange@redhat.com>
Subject: Re: [PATCH v6 3/4] net/slirp.c: Refactor address parsing
Date: Thu, 15 Apr 2021 08:36:32 -0700	[thread overview]
Message-ID: <CADPb22QwStkJ_M+E08fAid457+UxmCxdCQ86RgqwBRe3w_zHGQ@mail.gmail.com> (raw)
In-Reply-To: <20210415033925.1290401-4-dje@google.com>

[-- Attachment #1: Type: text/plain, Size: 2945 bytes --]

On Wed, Apr 14, 2021 at 8:40 PM Doug Evans <dje@google.com> wrote:

> ... in preparation for adding ipv6 host forwarding support.
>
> Tested:
> avocado run tests/acceptance/hostfwd.py
>
> Signed-off-by: Doug Evans <dje@google.com>
> ---
>
> [...]
>
> diff --git a/tests/acceptance/hostfwd.py b/tests/acceptance/hostfwd.py
> new file mode 100644
> index 0000000000..9b9db142c3
> --- /dev/null
> +++ b/tests/acceptance/hostfwd.py
> @@ -0,0 +1,91 @@
> [...]
> +
> +    def test_qmp_hostfwd_ipv4_parsing_errors(self):
> +        """Verify handling of various kinds of parsing errors."""
> +        self.vm.add_args('-nodefaults',
> +                         '-netdev', 'user,id=vnet',
> +                         '-device', 'virtio-net,netdev=vnet')
> +        self.vm.launch()
> +        self.assertEquals(self.hmc('hostfwd_remove abc::42'),
> +                          "Invalid format: bad protocol name 'abc'\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add abc::65022-:22'),
> +                          "Invalid host forwarding rule 'abc::65022-:22'"
> + \
> +                          " (bad protocol name 'abc')\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add :foo'),
> +                          "Invalid host forwarding rule ':foo'" + \
> +                          " (missing host-guest separator)\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add :a.b.c.d:66-:66'),
> +                          "Invalid host forwarding rule
> ':a.b.c.d:66-:66'" + \
> +                          " (For host address: address resolution failed
> for" \
> +                          " 'a.b.c.d:66': Name or service not known)\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add ::66-a.b.c.d:66'),
> +                          "Invalid host forwarding rule
> '::66-a.b.c.d:66'" + \
> +                          " (For guest address: address resolution
> failed" + \
> +                          " for 'a.b.c.d:66': Name or service not
> known)\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add ::-1-foo'),
> +                          "Invalid host forwarding rule '::-1-foo'" + \
> +                          " (For host address: error parsing port in" + \
> +                          " address ':')\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add ::66-foo'),
> +                          "Invalid host forwarding rule '::66-foo' (For"
> + \
> +                          " guest address: error parsing address
> 'foo')\r\n")
> +        self.assertEquals(self.hmc('hostfwd_add ::66-:0'),
> +                          "Invalid host forwarding rule '::66-:0'" + \
> +                          " (For guest address: invalid port '0')\r\n")
>

One improvement I think I'd like to make here is that I'm not sure how
portable the text of the result of, e.g., gai_strerror() is,
and relax the expected text of the error messages in the potentially
host-specific part.
But I'll wait until everything else is reviewed.

[-- Attachment #2: Type: text/html, Size: 4501 bytes --]