Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 8 May 2024 14:30:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Peter Eisentraut <peter@eisentraut.org>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Andrew Dunstan <andrew@dunslane.net>
Subject: Re: [PATCH] json_lex_string: don't overread on bad UTF8
Message-ID: <ZjsOA6R0q-BXcDc1@paquier.xyz>
References: <CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com>
 <df114cad-3ee0-42ea-8ee5-d8024dfa8a75@eisentraut.org>
 <CAOYmi+=BomJrQUBgy5FQY9ZtHvuK7WOJNB6foPUv21qfb2+YPw@mail.gmail.com>
 <ZjmjPyA29dIJjmjI@paquier.xyz>
 <CAOYmi+=yCFok+UNRHDJna5dSasqa9cMHviBZ6pYmtt1Yn_RfRg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="T9uO6tHG+jfv4sA1"
Content-Disposition: inline
In-Reply-To: <CAOYmi+=yCFok+UNRHDJna5dSasqa9cMHviBZ6pYmtt1Yn_RfRg@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZjsOA6R0q-BXcDc1%40paquier.xyz>
Precedence: bulk


--T9uO6tHG+jfv4sA1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, May 07, 2024 at 02:06:10PM -0700, Jacob Champion wrote:
> Maybe I've misunderstood, but isn't that what's being done in v2?

Something a bit different..  I was wondering if it could be possible
to tweak this code to truncate the data in the generated error string
so as the incomplete multi-byte sequence is entirely cut out, which
would come to setting token_terminator to "s" (last byte before the
incomplete byte sequence) rather than "term" (last byte available,
even if incomplete):
#define FAIL_AT_CHAR_END(code) \
do { \
   char       *term = s + pg_encoding_mblen(lex->input_encoding, s); \
   lex->token_terminator = (term <= end) ? term : s; \
   return code; \
} while (0)

But looking closer, I can see that in the JSON_INVALID_TOKEN case,
when !tok_done, we set token_terminator to point to the end of the
token, and that would include an incomplete byte sequence like in your
case.  :/

At the end of the day, I think that I'm OK with your patch and avoid
the overread for now in the back-branches.  This situation makes me
uncomfortable and we should put more effort in printing error messages
in a readable format, but that could always be tackled later as a
separate problem..  And I don't see something backpatchable at short
sight for v16.

Thoughts and/or objections?
--
Michael

--T9uO6tHG+jfv4sA1
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmY7DgMACgkQnvQgOdby
QH3/Bw//Ro2FSclAApcTu4gIR8mImxcM1OBiWgvYDPvQV9u1X0ObiA/IQ4mTrzqn
IW53hblo8Eh7emGViN5xD9JDlahJAOCGEr2DAYMW/3cksUwqXmsYdbs/Q3l58XkE
iL6DLiGfsCSI3hLuluNFbWOp74PFZj9/xt6WAnVrM8BcvtxhAim/qXqrpMRXJCDB
D3KM5tLRWhXLtGLraWuUUtwuGgBRf2o17z0NY3qBAuwcQuefdPQSwPYVogU5itMw
8fAk+e8wtKxD6FVRoIix66WGBKlwmc/CGegVaJzS+lS3UHT5/+KLsIf39CG1TF1X
C+qkcSEcXv3g/XX4isOUpWff8ISnmJYqhF6hKzYrUV9urCSLfNbwELqwvI0BOipr
0W+tf/Or7chIGggjijB+I2fVGhzrgroDbdKEoEA88VZDS9ihbSyHJ3+b75Q7t8op
nOHtSNF1aSyOZgww0y/6SFISstNW/FqS5vBM5BpzfbZw9XrntzxmbCvCZceQ/3mJ
/XoJVpjsmd1bi2r9xvPkq2JrU4vQYWPRLcULlFYYgWkezjVgZhzc3tEMD3Y7+QKJ
xYk4N5nDGSVlVTlFJCyclRnW5ULCeB60kqIKd8oiGvPo0AzrbiNgn0MnG9JNi3/R
2gdPNrfEQ3TPH81YByXavTLXE+NLCPWe9eNR7soG+AS31L2Q0qI=
=aXLJ
-----END PGP SIGNATURE-----

--T9uO6tHG+jfv4sA1--