MIME-Version: 1.0
From: Evan Jones <evan.jones@datadoghq.com>
Date: Mon, 5 Jun 2023 11:26:56 -0400
Message-ID: <CA+HWA9awUW0+RV_gO9r1ABZwGoZxPztcJxPy8vMFSTbTfi4jig@mail.gmail.com>
Subject: [PATCH] hstore: Fix parsing on Mac OS X: isspace() is locale specific
To: pgsql-hackers@postgresql.org
Content-Type: multipart/mixed; boundary="0000000000006922ae05fd638900"
Archived-At: <https://www.postgresql.org/message-id/CA%2BHWA9awUW0%2BRV_gO9r1ABZwGoZxPztcJxPy8vMFSTbTfi4jig%40mail.gmail.com>
Precedence: bulk

--0000000000006922ae05fd638900
Content-Type: multipart/alternative; boundary="0000000000006922ad05fd6389fe"

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

This patch fixes a rare parsing bug with unicode characters on Mac OS X.
The problem is that isspace() on Mac OS X changes its behaviour with the
locale. Use scanner_isspace instead, which only returns true for ASCII
whitespace. It appears other places in the Postgres code have already run
into this, since a number of places use scanner_isspace instead. However,
there are still a lot of other calls to isspace(). I'll try to take a quick
look to see if there might be other instances of this bug.

The bug is that in the following hstore value, the unicode character
"disappears", and is replaced with "key\xc4", because it is parsed
incorrectly:

select E'key=C4=85=3D>value'::hstore;
     hstore
-----------------
 "key=C4=85"=3D>"value"
(1 row)

select 'key=C4=85=3D>value'::hstore::text::bytea;
              bytea
----------------------------------
 \x226b6579c4223d3e2276616c756522
(1 row)

The correct result should be:

     hstore
-----------------
 "key=C4=85"=3D>"value"
(1 row)

That query is added to the regression test. The query works on Linux, but
failed on Mac OS X.

For a more detailed explanation of how isspace() works, on Mac OS X, see:
https://github.com/evanj/isspace_locale

Thanks!

Evan Jones

--0000000000006922ad05fd6389fe
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">This patch fixes a rare parsing bug with unicode character=
s on Mac OS X. The problem is that isspace() on Mac OS X changes its behavi=
our with the locale. Use scanner_isspace instead, which only returns true f=
or ASCII whitespace. It appears other places in the Postgres code have alre=
ady run into this, since a number of places=C2=A0use scanner_isspace instea=
d. However, there are still a lot of other calls to isspace(). I&#39;ll try=
 to take a quick look to see if there might be other instances of this bug.=
<div><br></div><div>The bug is that in the following hstore value, the unic=
ode character &quot;disappears&quot;, and is replaced with &quot;key\xc4&qu=
ot;, because it is parsed incorrectly:</div><div><br></div><div>select E=
9;key=C4=85=3D&gt;value&#39;::hstore;<br>=C2=A0 =C2=A0 =C2=A0hstore =C2=A0 =
=C2=A0 =C2=A0<br>-----------------<br>=C2=A0&quot;key=C4=85&quot;=3D&gt;&qu=
ot;value&quot;<br>(1 row)<br></div><div><br></div><div>select &#39;key=C4=
=85=3D&gt;value&#39;::hstore::text::bytea;<br></div><div>=C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 bytea =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 <br>----------------------------------<br>=C2=A0\x226b6579c42=
23d3e2276616c756522<br>(1 row)<br></div><div><br></div><div>The correct res=
ult=C2=A0should be:</div><div><br></div><div>=C2=A0 =C2=A0 =C2=A0hstore =C2=
=A0 =C2=A0 =C2=A0<br>-----------------<br>=C2=A0&quot;key=C4=85&quot;=3D&gt=
;&quot;value&quot;<br>(1 row)<br></div><div><br></div><div>That query is ad=
ded to the regression test. The query works on Linux, but failed on Mac OS =
X.</div><div><br></div><div>For a more detailed=C2=A0explanation of how iss=
pace() works, on Mac OS X, see:=C2=A0<a href=3D"https://github.com/evanj/is=
space_locale">https://github.com/evanj/isspace_locale</a></div><div><br></d=
iv><div>Thanks!</div><div><br>Evan Jones</div><div><br></div></div>

--0000000000006922ad05fd6389fe--
--0000000000006922ae05fd638900
Content-Type: application/octet-stream; name="hstore-isspace.patch"
Content-Disposition: attachment; filename="hstore-isspace.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lij08kl50>
X-Attachment-Id: f_lij08kl50

ZGlmZiAtLWdpdCBhL2NvbnRyaWIvaHN0b3JlL2V4cGVjdGVkL2hzdG9yZS5vdXQgYi9jb250cmli
L2hzdG9yZS9leHBlY3RlZC9oc3RvcmUub3V0CmluZGV4IDE4MzZjOWFjZjMuLjI2ODE1Y2JjN2Yg
MTAwNjQ0Ci0tLSBhL2NvbnRyaWIvaHN0b3JlL2V4cGVjdGVkL2hzdG9yZS5vdXQKKysrIGIvY29u
dHJpYi9oc3RvcmUvZXhwZWN0ZWQvaHN0b3JlLm91dApAQCAtMjQzLDYgKzI0MywxNCBAQCBzZWxl
Y3QgJwknOjpoc3RvcmU7CiAgCiAoMSByb3cpCiAKKy0tIFVURi04IE1hYyBPUyBYIGVuX1VTLlVU
Ri04IGxvY2FsZSBidWc6IGlzc3BhY2UoMHg4NSkgcmV0dXJucyB0cnVlCistLSBcdTAxMDUgZW5j
b2RlcyBhcyAweGM0IDB4ODUgaW4gVVRGLTg7IHRoZSAweDg1IHdhcyBpbnRlcnByZXRlZCBhcyB3
aGl0ZXNwYWNlCitzZWxlY3QgRSdrZXlcdTAxMDU9PnZhbHVlJzo6aHN0b3JlOworICAgICBoc3Rv
cmUgICAgICAKKy0tLS0tLS0tLS0tLS0tLS0tCisgImtlecSFIj0+InZhbHVlIgorKDEgcm93KQor
CiAtLSBpbnZhbGlkIGlucHV0CiBzZWxlY3QgJyAgPT5udWxsJzo6aHN0b3JlOwogRVJST1I6ICBz
eW50YXggZXJyb3IgaW4gaHN0b3JlLCBuZWFyICI9IiBhdCBwb3NpdGlvbiAyCmRpZmYgLS1naXQg
YS9jb250cmliL2hzdG9yZS9oc3RvcmVfaW8uYyBiL2NvbnRyaWIvaHN0b3JlL2hzdG9yZV9pby5j
CmluZGV4IGNlYzdkZjcxYTIuLjk5OWRkYWQ3NmQgMTAwNjQ0Ci0tLSBhL2NvbnRyaWIvaHN0b3Jl
L2hzdG9yZV9pby5jCisrKyBiL2NvbnRyaWIvaHN0b3JlL2hzdG9yZV9pby5jCkBAIC0xMyw2ICsx
Myw3IEBACiAjaW5jbHVkZSAibGliL3N0cmluZ2luZm8uaCIKICNpbmNsdWRlICJsaWJwcS9wcWZv
cm1hdC5oIgogI2luY2x1ZGUgIm5vZGVzL21pc2Nub2Rlcy5oIgorI2luY2x1ZGUgInBhcnNlci9z
Y2Fuc3VwLmgiCiAjaW5jbHVkZSAidXRpbHMvYnVpbHRpbnMuaCIKICNpbmNsdWRlICJ1dGlscy9q
c29uLmgiCiAjaW5jbHVkZSAidXRpbHMvanNvbmIuaCIKQEAgLTExOCw3ICsxMTksNyBAQCBnZXRf
dmFsKEhTUGFyc2VyICpzdGF0ZSwgYm9vbCBpZ25vcmVlcSwgYm9vbCAqZXNjYXBlZCkKIAkJCXsK
IAkJCQlzdCA9IEdWX1dBSVRFU0NJTjsKIAkJCX0KLQkJCWVsc2UgaWYgKCFpc3NwYWNlKCh1bnNp
Z25lZCBjaGFyKSAqKHN0YXRlLT5wdHIpKSkKKwkJCWVsc2UgaWYgKCFzY2FubmVyX2lzc3BhY2Uo
KHVuc2lnbmVkIGNoYXIpICooc3RhdGUtPnB0cikpKQogCQkJewogCQkJCSooc3RhdGUtPmN1cikg
PSAqKHN0YXRlLT5wdHIpOwogCQkJCXN0YXRlLT5jdXIrKzsKQEAgLTE0MSw3ICsxNDIsNyBAQCBn
ZXRfdmFsKEhTUGFyc2VyICpzdGF0ZSwgYm9vbCBpZ25vcmVlcSwgYm9vbCAqZXNjYXBlZCkKIAkJ
CQlzdGF0ZS0+cHRyLS07CiAJCQkJcmV0dXJuIHRydWU7CiAJCQl9Ci0JCQllbHNlIGlmIChpc3Nw
YWNlKCh1bnNpZ25lZCBjaGFyKSAqKHN0YXRlLT5wdHIpKSkKKwkJCWVsc2UgaWYgKHNjYW5uZXJf
aXNzcGFjZSgodW5zaWduZWQgY2hhcikgKihzdGF0ZS0+cHRyKSkpCiAJCQl7CiAJCQkJcmV0dXJu
IHRydWU7CiAJCQl9CkBAIC0yNTUsNyArMjU2LDcgQEAgcGFyc2VfaHN0b3JlKEhTUGFyc2VyICpz
dGF0ZSkKIAkJCXsKIAkJCQlQUlNFT0Y7CiAJCQl9Ci0JCQllbHNlIGlmICghaXNzcGFjZSgodW5z
aWduZWQgY2hhcikgKihzdGF0ZS0+cHRyKSkpCisJCQllbHNlIGlmICghc2Nhbm5lcl9pc3NwYWNl
KCh1bnNpZ25lZCBjaGFyKSAqKHN0YXRlLT5wdHIpKSkKIAkJCXsKIAkJCQlQUlNTWU5UQVhFUlJP
UjsKIAkJCX0KQEAgLTMwOSw3ICszMTAsNyBAQCBwYXJzZV9oc3RvcmUoSFNQYXJzZXIgKnN0YXRl
KQogCQkJewogCQkJCXJldHVybiB0cnVlOwogCQkJfQotCQkJZWxzZSBpZiAoIWlzc3BhY2UoKHVu
c2lnbmVkIGNoYXIpICooc3RhdGUtPnB0cikpKQorCQkJZWxzZSBpZiAoIXNjYW5uZXJfaXNzcGFj
ZSgodW5zaWduZWQgY2hhcikgKihzdGF0ZS0+cHRyKSkpCiAJCQl7CiAJCQkJUFJTU1lOVEFYRVJS
T1I7CiAJCQl9CmRpZmYgLS1naXQgYS9jb250cmliL2hzdG9yZS9zcWwvaHN0b3JlLnNxbCBiL2Nv
bnRyaWIvaHN0b3JlL3NxbC9oc3RvcmUuc3FsCmluZGV4IGVmZWY5MTI5MmEuLjM2Yzc0MjIwMTgg
MTAwNjQ0Ci0tLSBhL2NvbnRyaWIvaHN0b3JlL3NxbC9oc3RvcmUuc3FsCisrKyBiL2NvbnRyaWIv
aHN0b3JlL3NxbC9oc3RvcmUuc3FsCkBAIC01Myw2ICs1MywxMCBAQCBzZWxlY3QgZSdcXCJhPT5x
IncnOjpoc3RvcmU7CiBzZWxlY3QgJyc6OmhzdG9yZTsKIHNlbGVjdCAnCSc6OmhzdG9yZTsKIAor
LS0gVVRGLTggTWFjIE9TIFggZW5fVVMuVVRGLTggbG9jYWxlIGJ1ZzogaXNzcGFjZSgweDg1KSBy
ZXR1cm5zIHRydWUKKy0tIFx1MDEwNSBlbmNvZGVzIGFzIDB4YzQgMHg4NSBpbiBVVEYtODsgdGhl
IDB4ODUgd2FzIGludGVycHJldGVkIGFzIHdoaXRlc3BhY2UKK3NlbGVjdCBFJ2tleVx1MDEwNT0+
dmFsdWUnOjpoc3RvcmU7CisKIC0tIGludmFsaWQgaW5wdXQKIHNlbGVjdCAnICA9Pm51bGwnOjpo
c3RvcmU7CiBzZWxlY3QgJ2FhPT4iJzo6aHN0b3JlOwo=
--0000000000006922ae05fd638900--