Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 7 May 2024 12:42:55 +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: <ZjmjPyA29dIJjmjI@paquier.xyz>
References: <CAOYmi+ncM7pwLS3AnKCSmoqqtpjvA8wmCdoBtKA3ZrB2hZG6zA@mail.gmail.com>
 <df114cad-3ee0-42ea-8ee5-d8024dfa8a75@eisentraut.org>
 <CAOYmi+=BomJrQUBgy5FQY9ZtHvuK7WOJNB6foPUv21qfb2+YPw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="IruD35y1Xu5/6nhu"
Content-Disposition: inline
In-Reply-To: <CAOYmi+=BomJrQUBgy5FQY9ZtHvuK7WOJNB6foPUv21qfb2+YPw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/ZjmjPyA29dIJjmjI%40paquier.xyz>
Precedence: bulk


--IruD35y1Xu5/6nhu
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, May 03, 2024 at 07:05:38AM -0700, Jacob Champion wrote:
> On Fri, May 3, 2024 at 4:54=E2=80=AFAM Peter Eisentraut <peter@eisentraut=
=2Eorg> wrote:
>> but for the general encoding conversion we have what
>> would appear to be the same behavior in report_invalid_encoding(), and
>> we go out of our way there to produce a verbose error message including
>> the invalid data.

I was looking for that a couple of days ago in the backend but could
not put my finger on it.  Thanks.

> We could port something like that to src/common. IMO that'd be more
> suited for an actual conversion routine, though, as opposed to a
> parser that for the most part assumes you didn't lie about the input
> encoding and is just trying not to crash if you're wrong. Most of the
> time, the parser just copies bytes between delimiters around and it's
> up to the caller to handle encodings... the exceptions to that are the
> \uXXXX escapes and the error handling.

Hmm.  That would still leave the backpatch issue at hand, which is
kind of confusing to leave as it is.  Would it be complicated to
truncate the entire byte sequence in the error message and just give
up because we cannot do better if the input byte sequence is
incomplete?  We could still have some information depending on the
string given in input, which should be enough, hopefully.  With the
location pointing to the beginning of the sequence, even better.

> Offhand, are all of our supported frontend encodings
> self-synchronizing? By that I mean, is it safe to print a partial byte
> sequence if the locale isn't UTF-8? (As I type this I'm starting at
> Shift-JIS, and thinking "probably not.")
>
> Actually -- hopefully this is not too much of a tangent -- that
> further crystallizes a vague unease about the API that I have. The
> JsonLexContext is initialized with something called the
> "input_encoding", but that encoding is necessarily also the output
> encoding for parsed string literals and error messages. For the server
> side that's fine, but frontend clients have the input_encoding locked
> to UTF-8, which seems like it might cause problems? Maybe I'm missing
> code somewhere, but I don't see a conversion routine from
> json_errdetail() to the actual client/locale encoding. (And the parser
> does not support multibyte input_encodings that contain ASCII in trail
> bytes.)

Referring to json_lex_string() that does UTF-8 -> ASCII -> give-up in
its conversion for FRONTEND, I guess?  Yep.  This limitation looks
like a problem, especially if plugging that to libpq.
--
Michael

--IruD35y1Xu5/6nhu
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmY5oz8ACgkQnvQgOdby
QH0lCg/9FOiIin17mX6riAkgPZUU8Oiagf5nHRCwZfLvv1TyCCLS5R+gTHQP8oBq
d4E63iFdgGvoCsWY/ZVVwPiOkMtRRtxSskpQwyBqTGv5f40I/QKRIH9vaO4sNK7p
0xBZNZQhIwmofvPxXkSRFfxYS+45+cBtbj7IgPPr5oLa1oyjV1yvmUwvyd6CGnoL
GjmHYODB4kG/ibh35t0UGccCvFSz+NxC0aMS4zaSy4ISYQBkwrxoSYta/kY0nKcN
/5xuaFE6mwsnH2dzRxy5g4cA7HZv0NYqm4jrl70si8h2JabUtYkrq1GzUJNH7UdN
6ueN0+O3u6oztMxlPkeiWz5XU814zdX7O6WP25UdgZ4cfyiJ7AvsEWt6VS/VP1vI
VbJ/24Rw66GI9MWUynYj5ZWsew68O9HwUbN8sMPj9u7sbZUQsoBrubabaUztCret
2YMreq88S/1aSNy8cW5T+qG12QtCaAHt5Xwp5W/g49U54JXcl5wWlRn7qYp5hWkX
7zXEVm7yVc3sVbiANnJD028NSiaHAVMJ+SJiKs+UpNPuLDBmn9701tTrkImTvtYE
C7k1HZMZ3ifOqZ4kmOgYwwWIOVlqyt5UqjDgF9EaCTsfXAmsvEFjf7+QvwjRsU0M
ClHwU8mtfKaYWvXOw6QECtnBRI/wFP7ifp08kykQM5ET7Xy+/oc=
=f6z+
-----END PGP SIGNATURE-----

--IruD35y1Xu5/6nhu--