Message-ID: <91c0cdd2a402d59f8d6eaac40862d6f68e0b937c.camel@j-davis.com>
Subject: Re: Order changes in PG16 since ICU introduction
From: Jeff Davis <pgsql@j-davis.com>
To: Alexander Lakhin <exclusion@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Daniel Verite <daniel@manitou-mail.org>, Andrew Gierth
 <andrew@tao11.riddles.org.uk>, Peter Eisentraut
 <peter.eisentraut@enterprisedb.com>, Sandro Santilli <strk@kbt.io>, Regina
 Obe <lr@pcorp.us>, pgsql-hackers@lists.postgresql.org
Date: Mon, 15 May 2023 14:03:23 -0700
In-Reply-To: <2098874d-c111-41e4-9063-30bcf135226b@gmail.com>
References: <543e4ce6-8205-4aea-b54b-12b9ef747bf6@manitou-mail.org>
	 <654a49f7ff7461bcf47be4181430678d45f93858.camel@j-davis.com>
	 <37520ec1ae9591f83132f82dbd625f3fc2d69c16.camel@j-davis.com>
	 <483826.1683582475@sss.pgh.pa.us>
	 <aa6cac622417eda631a8a53287905fda1a963723.camel@j-davis.com>
	 <2098874d-c111-41e4-9063-30bcf135226b@gmail.com>
Content-Type: multipart/mixed; boundary="=-B3IMNaKXyoLfrbORUycm"
User-Agent: Evolution 3.44.4-0ubuntu1 
MIME-Version: 1.0
Archived-At: <https://www.postgresql.org/message-id/91c0cdd2a402d59f8d6eaac40862d6f68e0b937c.camel%40j-davis.com>
Precedence: bulk

--=-B3IMNaKXyoLfrbORUycm
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, 2023-05-13 at 13:00 +0300, Alexander Lakhin wrote:
> On the current master (after 455f948b0, and before f7faa9976, of
> course)
> I get an ASAN-detected failure with the following query:
> CREATE COLLATION col (provider =3D icu, locale =3D '123456789012');
>=20

Thank you for the report!

ICU source specifically says:

  /**                                                                =20
   * Useful constant for the maximum size of the language
     part of a locale ID.                                            =20
   * (including the terminating NULL).                               =20
   * @stable ICU 2.0                                                 =20
   */
  #define ULOC_LANG_CAPACITY 12

So the fact that it returning success without nul-terminating the
result is an ICU bug in my opinion, and I reported it here:

https://unicode-org.atlassian.net/browse/ICU-22394

Unfortunately that means we need to be a bit more paranoid and always
check for that warning, even if we have a preallocated buffer of the
"correct" size. It also means that both U_STRING_NOT_TERMINATED_WARNING
and U_BUFFER_OVERFLOW_ERROR will be user-facing errors (potentially
scary), unless we check for those errors each time and report specific
errors for them.

Another approach is to always check the length and loop using dynamic
allocation so that we never overflow (and forget about any static
buffers). That seems like overkill given that the problem case is
obviously invalid input; I think as long as we catch it and throw an
ERROR it's fine. But I can do this if others think it's worthwhile.

Patch attached. It just checks for the U_STRING_NOT_TERMINATED_WARNING
in a few places and turns it into an ERROR. It also cleans up the loop
for uloc_getLanguageTag() to check for U_STRING_NOT_TERMINATED_WARNING
rather than (U_SUCCESS(status) && len >=3D buflen).


--=20
Jeff Davis
PostgreSQL Contributor Team - AWS


--=-B3IMNaKXyoLfrbORUycm
Content-Disposition: attachment;
	filename="0001-ICU-check-for-U_STRING_NOT_TERMINATED_WARNING.patch"
Content-Type: text/x-patch;
	name="0001-ICU-check-for-U_STRING_NOT_TERMINATED_WARNING.patch";
	charset="UTF-8"
Content-Transfer-Encoding: base64

RnJvbSA5YzhlOTI3MmNhODA3YzlmNzVhN2IzMmZhMzE5MDcwMGNjNjAwMjYwIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBKZWZmIERhdmlzIDxqZWZmQGotZGF2aXMuY29tPgpEYXRlOiBN
b24sIDE1IE1heSAyMDIzIDEzOjM1OjA3IC0wNzAwClN1YmplY3Q6IFtQQVRDSF0gSUNVOiBjaGVj
ayBmb3IgVV9TVFJJTkdfTk9UX1RFUk1JTkFURURfV0FSTklORy4KCkluIHNvbWUgY2FzZXMsIElD
VSBjYW4gZmFpbCB0byBOVUwtdGVybWluYXRlIGEgcmVzdWx0IHN0cmluZyBldmVuIGlmCnVzaW5n
IGFuIGFwcHJvcHJpYXRlbHktc2l6ZWQgc3RhdGljIGJ1ZmZlci4gVGhlIGNhbGxlciBtdXN0IGVp
dGhlcgpjaGVjayBmb3IgbGVuID49IGJ1ZmxlbiBvciBVX1NUUklOR19OT1RfVEVSTUlOQVRFRF9X
QVJOSU5HLgoKVGhlIHNwZWNpZmljIHByb2JsZW0gaXMgcmVsYXRlZCB0byB1bG9jX2dldExhbmd1
YWdlKCksIGJ1dCBhZGQgdGhlCmNoZWNrIGluIG90aGVyIGNhc2VzIGFzIHdlbGwuCgpSZXBvcnRl
ZC1ieTogQWxleGFuZGVyIExha2hpbgpEaXNjdXNzaW9uOiBodHRwczovL3Bvc3Rnci5lcy9tLzIw
OTg4NzRkLWMxMTEtNDFlNC05MDYzLTMwYmNmMTM1MjI2YkBnbWFpbC5jb20KLS0tCiBzcmMvYmFj
a2VuZC91dGlscy9hZHQvcGdfbG9jYWxlLmMgfCAyOSArKysrKysrKysrKy0tLS0tLS0tLS0tLS0t
LS0tLQogc3JjL2Jpbi9pbml0ZGIvaW5pdGRiLmMgICAgICAgICAgIHwgMTUgKysrKy0tLS0tLS0t
LS0tCiAyIGZpbGVzIGNoYW5nZWQsIDE1IGluc2VydGlvbnMoKyksIDI5IGRlbGV0aW9ucygtKQoK
ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3V0aWxzL2FkdC9wZ19sb2NhbGUuYyBiL3NyYy9iYWNr
ZW5kL3V0aWxzL2FkdC9wZ19sb2NhbGUuYwppbmRleCBmMGI2NTY3ZGExLi4xY2Y5M2IyZDIwIDEw
MDY0NAotLS0gYS9zcmMvYmFja2VuZC91dGlscy9hZHQvcGdfbG9jYWxlLmMKKysrIGIvc3JjL2Jh
Y2tlbmQvdXRpbHMvYWR0L3BnX2xvY2FsZS5jCkBAIC0yNDY4LDcgKzI0NjgsNyBAQCBwZ191Y29s
X29wZW4oY29uc3QgY2hhciAqbG9jX3N0cikKIAogCQlzdGF0dXMgPSBVX1pFUk9fRVJST1I7CiAJ
CXVsb2NfZ2V0TGFuZ3VhZ2UobG9jX3N0ciwgbGFuZywgVUxPQ19MQU5HX0NBUEFDSVRZLCAmc3Rh
dHVzKTsKLQkJaWYgKFVfRkFJTFVSRShzdGF0dXMpKQorCQlpZiAoVV9GQUlMVVJFKHN0YXR1cykg
fHwgc3RhdHVzID09IFVfU1RSSU5HX05PVF9URVJNSU5BVEVEX1dBUk5JTkcpCiAJCXsKIAkJCWVy
ZXBvcnQoRVJST1IsCiAJCQkJCShlcnJtc2coImNvdWxkIG5vdCBnZXQgbGFuZ3VhZ2UgZnJvbSBs
b2NhbGUgXCIlc1wiOiAlcyIsCkBAIC0yNTA0LDcgKzI1MDQsNyBAQCBwZ191Y29sX29wZW4oY29u
c3QgY2hhciAqbG9jX3N0cikKIAkJICogUHJldGVuZCB0aGUgZXJyb3IgY2FtZSBmcm9tIHVjb2xf
b3BlbigpLCBmb3IgY29uc2lzdGVudCBlcnJvcgogCQkgKiBtZXNzYWdlIGFjcm9zcyBJQ1UgdmVy
c2lvbnMuCiAJCSAqLwotCQlpZiAoVV9GQUlMVVJFKHN0YXR1cykpCisJCWlmIChVX0ZBSUxVUkUo
c3RhdHVzKSB8fCBzdGF0dXMgPT0gVV9TVFJJTkdfTk9UX1RFUk1JTkFURURfV0FSTklORykKIAkJ
ewogCQkJdWNvbF9jbG9zZShjb2xsYXRvcik7CiAJCQllcmVwb3J0KEVSUk9SLApAQCAtMjYzOSw3
ICsyNjM5LDggQEAgaWN1X2Zyb21fdWNoYXIoY2hhciAqKnJlc3VsdCwgY29uc3QgVUNoYXIgKmJ1
ZmZfdWNoYXIsIGludDMyX3QgbGVuX3VjaGFyKQogCXN0YXR1cyA9IFVfWkVST19FUlJPUjsKIAls
ZW5fcmVzdWx0ID0gdWNudl9mcm9tVUNoYXJzKGljdV9jb252ZXJ0ZXIsICpyZXN1bHQsIGxlbl9y
ZXN1bHQgKyAxLAogCQkJCQkJCQkgYnVmZl91Y2hhciwgbGVuX3VjaGFyLCAmc3RhdHVzKTsKLQlp
ZiAoVV9GQUlMVVJFKHN0YXR1cykpCisJaWYgKFVfRkFJTFVSRShzdGF0dXMpIHx8CisJCXN0YXR1
cyA9PSBVX1NUUklOR19OT1RfVEVSTUlOQVRFRF9XQVJOSU5HKQogCQllcmVwb3J0KEVSUk9SLAog
CQkJCShlcnJtc2coIiVzIGZhaWxlZDogJXMiLCAidWNudl9mcm9tVUNoYXJzIiwKIAkJCQkJCXVf
ZXJyb3JOYW1lKHN0YXR1cykpKSk7CkBAIC0yNjgxLDcgKzI2ODIsNyBAQCBpY3Vfc2V0X2NvbGxh
dGlvbl9hdHRyaWJ1dGVzKFVDb2xsYXRvciAqY29sbGF0b3IsIGNvbnN0IGNoYXIgKmxvYywKIAlp
Y3VfbG9jYWxlX2lkID0gcGFsbG9jKGxlbiArIDEpOwogCSpzdGF0dXMgPSBVX1pFUk9fRVJST1I7
CiAJbGVuID0gdWxvY19jYW5vbmljYWxpemUobG9jLCBpY3VfbG9jYWxlX2lkLCBsZW4gKyAxLCBz
dGF0dXMpOwotCWlmIChVX0ZBSUxVUkUoKnN0YXR1cykpCisJaWYgKFVfRkFJTFVSRSgqc3RhdHVz
KSB8fCAqc3RhdHVzID09IFVfU1RSSU5HX05PVF9URVJNSU5BVEVEX1dBUk5JTkcpCiAJCXJldHVy
bjsKIAogCWxvd2VyX3N0ciA9IGFzY190b2xvd2VyKGljdV9sb2NhbGVfaWQsIHN0cmxlbihpY3Vf
bG9jYWxlX2lkKSk7CkBAIC0yNzY1LDcgKzI3NjYsNiBAQCBpY3Vfc2V0X2NvbGxhdGlvbl9hdHRy
aWJ1dGVzKFVDb2xsYXRvciAqY29sbGF0b3IsIGNvbnN0IGNoYXIgKmxvYywKIAogCXBmcmVlKGxv
d2VyX3N0cik7CiB9Ci0KICNlbmRpZgogCiAvKgpAQCAtMjc4OSw3ICsyNzg5LDcgQEAgaWN1X2xh
bmd1YWdlX3RhZyhjb25zdCBjaGFyICpsb2Nfc3RyLCBpbnQgZWxldmVsKQogCiAJc3RhdHVzID0g
VV9aRVJPX0VSUk9SOwogCXVsb2NfZ2V0TGFuZ3VhZ2UobG9jX3N0ciwgbGFuZywgVUxPQ19MQU5H
X0NBUEFDSVRZLCAmc3RhdHVzKTsKLQlpZiAoVV9GQUlMVVJFKHN0YXR1cykpCisJaWYgKFVfRkFJ
TFVSRShzdGF0dXMpIHx8IHN0YXR1cyA9PSBVX1NUUklOR19OT1RfVEVSTUlOQVRFRF9XQVJOSU5H
KQogCXsKIAkJaWYgKGVsZXZlbCA+IDApCiAJCQllcmVwb3J0KGVsZXZlbCwKQEAgLTI4MTEsMTkg
KzI4MTEsMTIgQEAgaWN1X2xhbmd1YWdlX3RhZyhjb25zdCBjaGFyICpsb2Nfc3RyLCBpbnQgZWxl
dmVsKQogCWxhbmd0YWcgPSBwYWxsb2MoYnVmbGVuKTsKIAl3aGlsZSAodHJ1ZSkKIAl7Ci0JCWlu
dDMyX3QJCWxlbjsKLQogCQlzdGF0dXMgPSBVX1pFUk9fRVJST1I7Ci0JCWxlbiA9IHVsb2NfdG9M
YW5ndWFnZVRhZyhsb2Nfc3RyLCBsYW5ndGFnLCBidWZsZW4sIHN0cmljdCwgJnN0YXR1cyk7CisJ
CXVsb2NfdG9MYW5ndWFnZVRhZyhsb2Nfc3RyLCBsYW5ndGFnLCBidWZsZW4sIHN0cmljdCwgJnN0
YXR1cyk7CiAKLQkJLyoKLQkJICogSWYgdGhlIHJlc3VsdCBmaXRzIGluIHRoZSBidWZmZXIgZXhh
Y3RseSAobGVuID09IGJ1ZmxlbiksCi0JCSAqIHVsb2NfdG9MYW5ndWFnZVRhZygpIHdpbGwgcmV0
dXJuIHN1Y2Nlc3Mgd2l0aG91dCBudWwtdGVybWluYXRpbmcKLQkJICogdGhlIHJlc3VsdC4gQ2hl
Y2sgZm9yIGVpdGhlciBVX0JVRkZFUl9PVkVSRkxPV19FUlJPUiBvciBsZW4gPj0KLQkJICogYnVm
bGVuIGFuZCB0cnkgYWdhaW4uCi0JCSAqLworCQkvKiB0cnkgYWdhaW4gaWYgdGhlIGJ1ZmZlciBp
cyBub3QgbGFyZ2UgZW5vdWdoICovCiAJCWlmICgoc3RhdHVzID09IFVfQlVGRkVSX09WRVJGTE9X
X0VSUk9SIHx8Ci0JCQkgKFVfU1VDQ0VTUyhzdGF0dXMpICYmIGxlbiA+PSBidWZsZW4pKSAmJgor
CQkJIHN0YXR1cyA9PSBVX1NUUklOR19OT1RfVEVSTUlOQVRFRF9XQVJOSU5HKSAmJgogCQkJYnVm
bGVuIDwgTWF4QWxsb2NTaXplKQogCQl7CiAJCQlidWZsZW4gPSBNaW4oYnVmbGVuICogMiwgTWF4
QWxsb2NTaXplKTsKQEAgLTI4NzgsNyArMjg3MSw3IEBAIGljdV92YWxpZGF0ZV9sb2NhbGUoY29u
c3QgY2hhciAqbG9jX3N0cikKIAkvKiB2YWxpZGF0ZSB0aGF0IHdlIGNhbiBleHRyYWN0IHRoZSBs
YW5ndWFnZSAqLwogCXN0YXR1cyA9IFVfWkVST19FUlJPUjsKIAl1bG9jX2dldExhbmd1YWdlKGxv
Y19zdHIsIGxhbmcsIFVMT0NfTEFOR19DQVBBQ0lUWSwgJnN0YXR1cyk7Ci0JaWYgKFVfRkFJTFVS
RShzdGF0dXMpKQorCWlmIChVX0ZBSUxVUkUoc3RhdHVzKSB8fCBzdGF0dXMgPT0gVV9TVFJJTkdf
Tk9UX1RFUk1JTkFURURfV0FSTklORykKIAl7CiAJCWVyZXBvcnQoZWxldmVsLAogCQkJCShlcnJt
c2coImNvdWxkIG5vdCBnZXQgbGFuZ3VhZ2UgZnJvbSBJQ1UgbG9jYWxlIFwiJXNcIjogJXMiLApA
QCAtMjkwMSw3ICsyODk0LDcgQEAgaWN1X3ZhbGlkYXRlX2xvY2FsZShjb25zdCBjaGFyICpsb2Nf
c3RyKQogCiAJCXN0YXR1cyA9IFVfWkVST19FUlJPUjsKIAkJdWxvY19nZXRMYW5ndWFnZShvdGhl
cmxvYywgb3RoZXJsYW5nLCBVTE9DX0xBTkdfQ0FQQUNJVFksICZzdGF0dXMpOwotCQlpZiAoVV9G
QUlMVVJFKHN0YXR1cykpCisJCWlmIChVX0ZBSUxVUkUoc3RhdHVzKSB8fCBzdGF0dXMgPT0gVV9T
VFJJTkdfTk9UX1RFUk1JTkFURURfV0FSTklORykKIAkJCWNvbnRpbnVlOwogCiAJCWlmIChzdHJj
bXAobGFuZywgb3RoZXJsYW5nKSA9PSAwKQpkaWZmIC0tZ2l0IGEvc3JjL2Jpbi9pbml0ZGIvaW5p
dGRiLmMgYi9zcmMvYmluL2luaXRkYi9pbml0ZGIuYwppbmRleCBlMDNkNDk4YjFlLi4zMGI1NzY5
MzJmIDEwMDY0NAotLS0gYS9zcmMvYmluL2luaXRkYi9pbml0ZGIuYworKysgYi9zcmMvYmluL2lu
aXRkYi9pbml0ZGIuYwpAQCAtMjI1Miw3ICsyMjUyLDcgQEAgaWN1X2xhbmd1YWdlX3RhZyhjb25z
dCBjaGFyICpsb2Nfc3RyKQogCiAJc3RhdHVzID0gVV9aRVJPX0VSUk9SOwogCXVsb2NfZ2V0TGFu
Z3VhZ2UobG9jX3N0ciwgbGFuZywgVUxPQ19MQU5HX0NBUEFDSVRZLCAmc3RhdHVzKTsKLQlpZiAo
VV9GQUlMVVJFKHN0YXR1cykpCisJaWYgKFVfRkFJTFVSRShzdGF0dXMpIHx8IHN0YXR1cyA9PSBV
X1NUUklOR19OT1RfVEVSTUlOQVRFRF9XQVJOSU5HKQogCXsKIAkJcGdfZmF0YWwoImNvdWxkIG5v
dCBnZXQgbGFuZ3VhZ2UgZnJvbSBsb2NhbGUgXCIlc1wiOiAlcyIsCiAJCQkJIGxvY19zdHIsIHVf
ZXJyb3JOYW1lKHN0YXR1cykpOwpAQCAtMjI3MiwxOSArMjI3MiwxMiBAQCBpY3VfbGFuZ3VhZ2Vf
dGFnKGNvbnN0IGNoYXIgKmxvY19zdHIpCiAJbGFuZ3RhZyA9IHBnX21hbGxvYyhidWZsZW4pOwog
CXdoaWxlICh0cnVlKQogCXsKLQkJaW50MzJfdAkJbGVuOwotCiAJCXN0YXR1cyA9IFVfWkVST19F
UlJPUjsKLQkJbGVuID0gdWxvY190b0xhbmd1YWdlVGFnKGxvY19zdHIsIGxhbmd0YWcsIGJ1Zmxl
biwgc3RyaWN0LCAmc3RhdHVzKTsKKwkJdWxvY190b0xhbmd1YWdlVGFnKGxvY19zdHIsIGxhbmd0
YWcsIGJ1Zmxlbiwgc3RyaWN0LCAmc3RhdHVzKTsKIAotCQkvKgotCQkgKiBJZiB0aGUgcmVzdWx0
IGZpdHMgaW4gdGhlIGJ1ZmZlciBleGFjdGx5IChsZW4gPT0gYnVmbGVuKSwKLQkJICogdWxvY190
b0xhbmd1YWdlVGFnKCkgd2lsbCByZXR1cm4gc3VjY2VzcyB3aXRob3V0IG51bC10ZXJtaW5hdGlu
ZwotCQkgKiB0aGUgcmVzdWx0LiBDaGVjayBmb3IgZWl0aGVyIFVfQlVGRkVSX09WRVJGTE9XX0VS
Uk9SIG9yIGxlbiA+PQotCQkgKiBidWZsZW4gYW5kIHRyeSBhZ2Fpbi4KLQkJICovCisJCS8qIHRy
eSBhZ2FpbiBpZiB0aGUgYnVmZmVyIGlzIG5vdCBsYXJnZSBlbm91Z2ggKi8KIAkJaWYgKHN0YXR1
cyA9PSBVX0JVRkZFUl9PVkVSRkxPV19FUlJPUiB8fAotCQkJKFVfU1VDQ0VTUyhzdGF0dXMpICYm
IGxlbiA+PSBidWZsZW4pKQorCQkJc3RhdHVzID09IFVfU1RSSU5HX05PVF9URVJNSU5BVEVEX1dB
Uk5JTkcpCiAJCXsKIAkJCWJ1ZmxlbiA9IGJ1ZmxlbiAqIDI7CiAJCQlsYW5ndGFnID0gcGdfcmVh
bGxvYyhsYW5ndGFnLCBidWZsZW4pOwotLSAKMi4zNC4xCgo=


--=-B3IMNaKXyoLfrbORUycm--