From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1FvO-0004ZL-Ex for qemu-devel@nongnu.org; Tue, 24 Nov 2015 10:54:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1FvL-00021j-7f for qemu-devel@nongnu.org; Tue, 24 Nov 2015 10:54:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44469) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1FvK-00021H-WE for qemu-devel@nongnu.org; Tue, 24 Nov 2015 10:54:31 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id E7722C10045D for ; Tue, 24 Nov 2015 15:54:29 +0000 (UTC) References: <1448377362-18117-1-git-send-email-berrange@redhat.com> <1448377362-18117-2-git-send-email-berrange@redhat.com> From: Eric Blake Message-ID: <56548830.6060200@redhat.com> Date: Tue, 24 Nov 2015 08:54:24 -0700 MIME-Version: 1.0 In-Reply-To: <1448377362-18117-2-git-send-email-berrange@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="eE5KTLcXklpGIc6WvlqxVBNh0RCLSRAaf" Subject: Re: [Qemu-devel] [PATCH v2 1/5] util: add base64 decoding function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Markus Armbruster This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --eE5KTLcXklpGIc6WvlqxVBNh0RCLSRAaf Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/24/2015 08:02 AM, Daniel P. Berrange wrote: > The standard glib provided g_base64_decode doesn't provide any > kind of sensible error checking on its input. Add a QEMU custom > wrapper qbase64_decode which can be used with untrustworthy > input that can contain invalid base64 characters, embedded > NUL characters, or not be NUL terminated at all. >=20 > Signed-off-by: Daniel P. Berrange > --- > +/** > + * qbase64_decode: > + * @input: the (possibly) base64 encoded text > + * @in_len: length of @input or -1 if NUL terminated > + * @out_len: filled with length of decoded data > + * @errpr: pointer to uninitialized error object s/errpr/errp/ > + * Returns: the decoded data or NULL > + */ > +uint8_t *qbase64_decode(const char *input, > + size_t in_len, > + size_t *out_len, > + Error **errp); > + Is char* any easier to work with than uint8_t* as the return type? I'm fine with either, and actually like that uint8_t doesn't cause unintentional sign-extension on 8-bit input, but just want to make sure we aren't forcing the majority of our callers to cast back to a more convenient type. > +static void test_base64_good(void) > +{ > + const char *input =3D "QmVjYXVzZSB3ZSBmb2N1c2VkIG9uIHRoZS" > + "BzbmFrZSwgd2UgbWlzc2VkIHRoZSBzY29ycGlvbi4=3D"; > + const char *expect =3D "Because we focused on the snake, " > + "we missed the scorpion."; > + > + size_t len; > + uint8_t *actual =3D qbase64_decode(input, > + -1, > + &len, > + &error_abort); > + > + g_assert(actual !=3D NULL); > + g_assert_cmpint(len, =3D=3D, strlen(expect)); > + g_assert_cmpstr((char *)actual, =3D=3D, expect); > + g_free(actual); For example, this demonstrates a caller having to cast. But it doesn't show whether more callers want char* or uint8_t*. > + > +static void test_base64_embedded_nul(void) > +{ > + const char input[] =3D "There's no such\0thing as a free lunch."; > + > + test_base64_bad(input, G_N_ELEMENTS(input) - 1); > +} > + > + > +static void test_base64_not_nul_terminated(void) > +{ > + char input[] =3D "There's no such\0thing as a free lunch."; > + input[G_N_ELEMENTS(input) - 1] =3D '!'; > + > + test_base64_bad(input, G_N_ELEMENTS(input) - 1); > +} > + Did you mean to have an embedded NUL in the second test, or should you change that \0 to space? > +++ b/util/base64.c > +#include "qemu/base64.h" > + > +static const char *base64_valid_chars =3D > + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=3D= "; Do we want to allow newlines (perhaps by adding a bool parameter)? After all, although newline is not valid in base64, it is the one character that GNU coreutils special-cases (produce on output every --wrap columns, ignore on input without needing --ignore-garbage) to make base64 blocks easier to read by breaking into lines rather than one long string - and which may be relevant if someone is pasting output from base64(1) into QMP. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --eE5KTLcXklpGIc6WvlqxVBNh0RCLSRAaf Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWVIgwAAoJEKeha0olJ0NqrJAH/0I/M76zNiqWMHWhdd1izKAd yCQPEciyoTbKijLviO+YlN7SVpry7OU0xAOfLrqT75DjiuZDwijuZEjDEhkv3Hin S9rBNvgXZNv0juGo1z/vygkoiz8jCMKuzAdUtvMvoZWkALf+LL9sTxvk1lfNbTqx tOdXrRkboVO7KvnHZMZy4opWBIO1oBdCd2czqEBN7D29BA5ZAdHbAxauqrJIB8KM sdTGLvnoOCmWNw/e2RUhBx/j3nB3v1BXqQ5JVNhmjLZ4StqOrCRrNjwnbmUp1v/l U8YipUNO4TOCufQ0xTbcpwYVF7Z43Ad+xlNkM0u2WRhsYQBtPqkJsA+oGAmPWKk= =MzUi -----END PGP SIGNATURE----- --eE5KTLcXklpGIc6WvlqxVBNh0RCLSRAaf--