From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1oNcGW-00023Q-6B for mharc-grub-devel@gnu.org; Mon, 15 Aug 2022 11:44:32 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53006) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oNcGT-00021x-10 for grub-devel@gnu.org; Mon, 15 Aug 2022 11:44:29 -0400 Received: from wout2-smtp.messagingengine.com ([64.147.123.25]:54801) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oNcGQ-0008CJ-QB for grub-devel@gnu.org; Mon, 15 Aug 2022 11:44:28 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 257A3320095A; Mon, 15 Aug 2022 11:44:23 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 15 Aug 2022 11:44:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm1; t=1660578262; x=1660664662; bh=QHGgygaj6K seda2hgrmMh69XgjtsSdHf0Kc1U0BPZUw=; b=iBbyCCx7J37Uu57fY+see/EHj2 gOC4vo8Wgz1NSI0vEbfutBB6jAyzlI3x5t8TYyAklY7XeW9t8XOWytcln044x7hL IYVLuS172SZgTHqQ3Z2w1Y9cR1lNQw/uTOxt4GzdN6s7uvc89+NCbCOyRWPrLjES jA+YnmBd7jlSfjdIe5s+T2xjyt4NL5AvV2D/Nl8GSPc60ESQXuIHmN3AIR09Yrm+ s4M3K70jCEbxQDQf2WR5vZimWYr2NHMnmu5mNmxEOTHBwvBky6z66qVSjM3gffl7 /q5JHlF6Y5QxjNqXmWIiIKxftc9m18HjKhIFHKLaCivAeC9W4wRqTq+uDCww== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1660578262; x=1660664662; bh=QHGgygaj6Kseda2hgrmMh69Xgjts SdHf0Kc1U0BPZUw=; b=Kk99EjUKh9R8aXAzMRVAK2YjkakbkVImX4kc2qCjnyPt c8SvIEBML5kaw9Y9Fged+DsqOz8qD77ptLWc/bXLkq6ov5sM2mdL+4aeyCg7DkMf 2+54TZmTKqm8Lm5+kTuEcaxOLfNaDiv61vmY2LXGlJkzyEe/+Hei/lHeXXwlrlM1 2TpLds07uRhA1yLKX/OrgbW+1o+nK87nK6vr0l05TVhLbXGBLdu3IMe7N06JrHjy jz7aELsJhYU06MH7Fr81z9sa/eZhZl80hpV6Ih32rfd021bwgfikYa47EY/4GftK hoNbm+uDFgKWvDalEjPp6IV9qn5wYTUhRN2pzoYj6A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrvdehvddgleefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhephfeiieeuvdffveeigeejjeffkeektdehleekvdekheeilefgkeefvdegtdekfeei necuffhomhgrihhnpehgnhhurdhorhhgnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 15 Aug 2022 11:44:21 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id e3961546 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 15 Aug 2022 15:44:16 +0000 (UTC) Date: Mon, 15 Aug 2022 17:44:19 +0200 From: Patrick Steinhardt To: The development of GNU GRUB Cc: Nicholas Vinson Subject: Re: [PATCH v5 1/2] json: Add function to unescape JSON-encoded strings Message-ID: References: <20220712133913.ze6dkcbrlp5q5hkq@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="T8lZRVJq1olonWKZ" Content-Disposition: inline In-Reply-To: <20220712133913.ze6dkcbrlp5q5hkq@tomti.i.net-space.pl> Received-SPF: pass client-ip=64.147.123.25; envelope-from=ps@pks.im; helo=wout2-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Aug 2022 15:44:29 -0000 --T8lZRVJq1olonWKZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jul 12, 2022 at 03:39:13PM +0200, Daniel Kiper wrote: > On Mon, Jul 11, 2022 at 09:08:09AM -0400, Nicholas Vinson wrote: > > On 7/11/22 06:44, Patrick Steinhardt wrote: > > > JSON strings require certain characters to be encoded, either by usin= g a > > > single reverse solidus character "\" for a set of popular characters,= or > > > by using a Unicode representation of "\uXXXXX". The jsmn library does= n't > > > handle unescaping for us, so we must implement this functionality for > > > ourselves. > > > > > > Add a new function `grub_json_unescape ()` that takes a potentially > > > escaped JSON string as input and returns a new unescaped string. > > > > > > Signed-off-by: Patrick Steinhardt > > > --- > > > grub-core/lib/json/json.c | 118 +++++++++++++++++++++++++++++++++++= +++ > > > grub-core/lib/json/json.h | 12 ++++ > > > 2 files changed, 130 insertions(+) > > > > > > diff --git a/grub-core/lib/json/json.c b/grub-core/lib/json/json.c > > > index 1c20c75ea..1eadd1ce9 100644 > > > --- a/grub-core/lib/json/json.c > > > +++ b/grub-core/lib/json/json.c > > > @@ -262,3 +262,121 @@ grub_json_getint64 (grub_int64_t *out, const gr= ub_json_t *parent, const char *ke > > > return GRUB_ERR_NONE; > > > } > > > + > > > +grub_err_t > > > +grub_json_unescape (char **out, grub_size_t *outlen, const char *in,= grub_size_t inlen) > > > +{ > > > + grub_err_t ret =3D GRUB_ERR_NONE; > > > + grub_size_t inpos, resultpos; > > > + char *result; > > > + > > > + if (out =3D=3D NULL || outlen =3D=3D NULL) > > > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("output parameters = are not set")); > > > + > > > + result =3D grub_calloc (1, inlen + 1); > > > + if (result =3D=3D NULL) > > > + return GRUB_ERR_OUT_OF_MEMORY; > > > + > > > + for (inpos =3D resultpos =3D 0; inpos < inlen; inpos++) > > > + { > > > + if (in[inpos] =3D=3D '\\') > > > + { > > > + inpos++; > > > + if (inpos >=3D inlen) > > > + { > > > + ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, N_("expected escap= ed character")); > > > + goto err; > > > + } > > > + > > > + switch (in[inpos]) > > > + { > > > + case '"': > > > + result[resultpos++] =3D '"'; > > > + break; > > > + > > > + case '/': > > > + result[resultpos++] =3D '/'; > > > + break; > > > + > > > + case '\\': > > > + result[resultpos++] =3D '\\'; > > > + break; > > > + > > > + case 'b': > > > + result[resultpos++] =3D '\b'; > > > + break; > > > + > > > + case 'f': > > > + result[resultpos++] =3D '\f'; > > > + break; > > > + > > > + case 'r': > > > + result[resultpos++] =3D '\r'; > > > + break; > > > + > > > + case 'n': > > > + result[resultpos++] =3D '\n'; > > > + break; > > > + > > > + case 't': > > > + result[resultpos++] =3D '\t'; > > > + break; > > > + > > > + case 'u': > > > + { > > > + char values[4] =3D {0}; > > > + unsigned i; > > > + > > > + inpos++; > > > + if (inpos + ARRAY_SIZE(values) > inlen) > > > + { > > > + ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unicode seque= nce too short")); > > > + goto err; > > > + } > > > > I recommend relaxing these errors a little bit. While technically > > correct, I'm thinking it might serve GRUB better to be a bit more > > forgiving. > > > > Perhaps something like: > > > > if the input is '\u????' where ? is not a hex digit, set the value to > > 'u', and process ???? separately. > > > > if ???? is less than ARRAY_SIZE chars, left-pad ???? with 0s* until it's > > the correct length then convert the value. > > > > (* I don't mean to literally pad. I mean to treat the short number as if > > it was left-padded with 0s) > > > > > + > > > + for (i =3D 0; i < ARRAY_SIZE(values); i++) > > > + { > > > + char c =3D in[inpos++]; > > > + > > > + if (c >=3D '0' && c <=3D '9') > > > + values[i] =3D c - '0'; > > > + else if (c >=3D 'A' && c <=3D 'F') > > > + values[i] =3D c - 'A' + 10; > > > + else if (c >=3D 'a' && c <=3D 'f') > > > + values[i] =3D c - 'a' + 10; > > > + else > > > + { > > > > With a similar argument here. Treat the short ???? as suggested above > > and resume normal processing at the non-hex digit position. > > > > > + ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, > > > + N_("unicode sequence with invalid character '%c'"), c); > > > + goto err; > > > + } > > > + } > > > + > > > + if (values[0] !=3D 0 || values[1] !=3D 0) > > > + result[resultpos++] =3D values[0] << 4 | values[1]; > > > + result[resultpos++] =3D values[2] << 4 | values[3]; > > > + > > > + /* Offset the increment that's coming in via the loop increment.= */ > > > + inpos--; > > > + > > > + break; > > > + } > > > + > > > + default: > > > + ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized escape= d character '%c'"), in[inpos]); > > > + goto err; > > > > And the final prong of the suggestion would be here. Treat the improper > > escape as if it had not been escaped at all or as '\\?' where '?' is the > > escaped character. >=20 > Nicholas comments make sense for me. >=20 > Patrick? They do make sense indeed. But it feels like we're now iterating more on the way we decode JSON-strings, which is exactly the kind of reason why I initially wanted to punt on that and just decode only those characters that cryptsetup v2.0.0 would've written with an escape character back then. And this really only is the backslash character. I don't think it's sensible to make this decoding more complicated by handling all these different cases when we know that there only was a single version of cryptsetup that would only have written a single character with optional escaping, and that this case was changed at a later point in time to not do that anymore. So ultimately this all feels rather theoretical at this point, so I lean towards being pragmatic and making adjustments at a later point when we ever see that this is indeed a real-world problem. Patrick > Daniel >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel --T8lZRVJq1olonWKZ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmL6adIACgkQVbJhu7ck PpRL/Q//aCjNAnpp0jzEKQ0146dKM/sfqPW137kDmi1dMRT587oAzyLyspCm9hil 4QMmyNqW7ReK0STJckMuVF5mKfp/S0nqcao30XAp3N9PR1L0MFzKWgZDDWTW7oTh UhLvVfJNGuTOoBDY5hTNwxisYdeV6+ebvhdLpgKTtDdg+SaZr9gLrg19kAKlBCE9 AqaQ6+4MXib3hTxYiCObnyuuE/6oV1MyXgR0kiK9rIxcSdR0EpA768b7KIzgt8GN d/Yfv1NvM2B94j57uR8NfVg4RM3AIUYhkW+VJRL8/j3prelv+rGPxWN3NDmOhD4d oAcsGgd0W5zEBMOihbElCRABTwJjl1TdqojHlVp7u00jdnlbEdgQo3nN9Cs+oIIG nwwKBZYXI2bhtE90f1VDnqzQMW8PNiUDhkBNhKf98T2Az/uQazI4ji8rxdKqCBNY nQXxKK7hrbHWO/QR/rgb7kHjsE1pLr6KwXVBC6IiFEg0F9nNGRpWfC3B/n4r9TSJ fPIFYbQ2KykOPYbKqSb8RhLrPMtcEOA2i0gqiia6SiB8dhx9fKm91fC3z4UA2oyK O38EAobwxmuCTxDvj6g1HtBojSsUlTta61VPkm1HBSrNTAexBxgGkOvmZWl+pp/G IzKYe8WGrPevuxY5GO3n8oEqZrF+mLGm0y+NPIXTsdOb3c5xs/o= =T1xi -----END PGP SIGNATURE----- --T8lZRVJq1olonWKZ--