From d0c5397e2dda0387c1678a279bc13187c8dd455d Mon Sep 17 00:00:00 2001 From: Yorhel Date: Mon, 28 Apr 2025 10:20:53 +0200 Subject: [PATCH] json_parse()/pgtypes: Fix accidental creation of read-only array/hash values &PL_sv_* shouldn't be used when constructing arrays or hashes in this context. --- FU.xs | 6 ++++++ c/jsonparse.c | 4 ++-- c/pgst.c | 2 +- c/pgtypes.c | 12 ++++++++---- t/json_parse.t | 8 +++++++- t/pgconnect.t | 1 + t/pgtypes-dynamic.t | 7 ++++++- t/pgtypes.t | 16 ++++++++++++++-- 8 files changed, 45 insertions(+), 11 deletions(-) diff --git a/FU.xs b/FU.xs index 221740b..dc19870 100644 --- a/FU.xs +++ b/FU.xs @@ -20,6 +20,12 @@ #ifndef BOOL_INTERNALS_sv_isbool_true #define BOOL_INTERNALS_sv_isbool_true(x) SvTRUEx(x) #endif +#ifndef newSV_true +#define newSV_true() newSVsv(&PL_sv_yes) +#endif +#ifndef newSV_false +#define newSV_false() newSVsv(&PL_sv_no) +#endif /* Disable key/value struct packing in khashl, so we can safely take a pointer * to values inside the hash table. */ diff --git a/c/jsonparse.c b/c/jsonparse.c index d24b1f9..6dfee91 100644 --- a/c/jsonparse.c +++ b/c/jsonparse.c @@ -236,12 +236,12 @@ static SV *fujson_parse(pTHX_ fujson_parse_ctx *ctx) { if (ctx->end - ctx->buf < 4) return NULL; if (memcmp(ctx->buf, "true", 4) != 0) return NULL; ctx->buf += 4; - return &PL_sv_yes; + return newSV_true(); case 'f': if (ctx->end - ctx->buf < 5) return NULL; if (memcmp(ctx->buf, "false", 5) != 0) return NULL; ctx->buf += 5; - return &PL_sv_no; + return newSV_false(); case 'n': if (ctx->end - ctx->buf < 4) return NULL; if (memcmp(ctx->buf, "null", 4) != 0) return NULL; diff --git a/c/pgst.c b/c/pgst.c index e5a2a3d..e943450 100644 --- a/c/pgst.c +++ b/c/pgst.c @@ -463,7 +463,7 @@ static SV *fupg_st_kvv(pTHX_ fupg_st *st) { SAVETMPS; SV *key = sv_2mortal(fupg_st_getval(aTHX_ st, i, 0)); if (hv_exists_ent(hv, key, 0)) fu_confess("Key '%s' is duplicated in $st->kvv() query results", SvPV_nolen(key)); - hv_store_ent(hv, key, st->nfields == 1 ? &PL_sv_yes : fupg_st_getval(aTHX_ st, i, 1), 0); + hv_store_ent(hv, key, st->nfields == 1 ? newSV_true() : fupg_st_getval(aTHX_ st, i, 1), 0); FREETMPS; } return sv; diff --git a/c/pgtypes.c b/c/pgtypes.c index b307cc0..471b6d2 100644 --- a/c/pgtypes.c +++ b/c/pgtypes.c @@ -78,7 +78,7 @@ SENDFN(domain) { (void)out; SERR("domain type should not be handled by this func RECVFN(bool) { RLEN(1); - return *buf ? &PL_sv_yes : &PL_sv_no; + return *buf ? newSV_true() : newSV_false(); } SENDFN(bool) { @@ -89,7 +89,7 @@ SENDFN(bool) { RECVFN(void) { RLEN(0); (void)buf; - return &PL_sv_undef; + return newSV(0); } SENDFN(void) { @@ -269,7 +269,7 @@ SENDFN(jsonpath) { #define ARRAY_MAXDIM 100 static SV *fupg_recv_array_elem(pTHX_ const fupg_tio *elem, const char *header, U32 dim, U32 ndim, const char **buf, const char *end) { - SV *r = &PL_sv_undef; + SV *r; if (dim == ndim) { if (end - *buf < 4) fu_confess("Invalid array format"); I32 len = fu_frombeI(32, *buf); @@ -279,6 +279,8 @@ static SV *fupg_recv_array_elem(pTHX_ const fupg_tio *elem, const char *header, if (len >= 0) { r = elem->recv(aTHX_ elem, *buf, len); *buf += len; + } else { + r = newSV(0); } } else { @@ -403,12 +405,14 @@ RECVFN(record) { if (oid != ctx->record.info->attrs[i].oid) RERR("expected field %d to be of type %u but got %u", i, ctx->record.info->attrs[i].oid, oid); I32 vlen = fu_frombeI(32, buf+4); - SV *r = &PL_sv_undef; + SV *r; buf += 8; len -= 8; if (vlen > len) RERR("input data too short"); if (vlen >= 0) { r = ctx->record.tio[i].recv(aTHX_ ctx->record.tio+i, buf, vlen); buf += vlen; len -= vlen; + } else { + r = newSV(0); } hv_store(hv, ctx->record.info->attrs[i].name.n, -strlen(ctx->record.info->attrs[i].name.n), r, 0); } diff --git a/t/json_parse.t b/t/json_parse.t index 0c26dff..d01414f 100644 --- a/t/json_parse.t +++ b/t/json_parse.t @@ -2,7 +2,7 @@ use v5.36; use Test::More; use FU::Util 'json_parse'; no warnings 'experimental::builtin'; -use builtin 'is_bool', 'created_as_number'; +use builtin 'is_bool', 'created_as_number', 'true', 'false'; use Config; my @error = ( @@ -236,4 +236,10 @@ ok !eval { json_parse '{"":{"":{"":{"":1}}}}', max_depth => 4; 1 }; ok !eval { json_parse '"string"', max_size => 7 }; } +# Mutable hashes/arrays +my $d = json_parse('[true,false,null,{"a":true,"b":false,"c":null}]'); +is_deeply $d, [true,false,undef,{a => true, b => false, c => undef}]; +$_ = 1 for @{$d}[0,1,2], values $d->[3]->%*; +is_deeply $d, [1,1,1,{a => 1, b => 1, c => 1}]; + done_testing; diff --git a/t/pgconnect.t b/t/pgconnect.t index 798734c..8536574 100644 --- a/t/pgconnect.t +++ b/t/pgconnect.t @@ -197,6 +197,7 @@ subtest '$st->kvv', sub { is_deeply $conn->q('SELECT 1 WHERE false')->kvv, {}; is_deeply $conn->q('SELECT 1')->kvv, {1=>1}; is_deeply $conn->q('SELECT 1, null UNION ALL SELECT 3, 2')->kvv, {1=>undef,3=>2}; + $conn->q('SELECT 1')->kvv->{1} = 0; }; subtest '$st->kva', sub { diff --git a/t/pgtypes-dynamic.t b/t/pgtypes-dynamic.t index b6954b5..2751a86 100644 --- a/t/pgtypes-dynamic.t +++ b/t/pgtypes-dynamic.t @@ -96,10 +96,15 @@ subtest 'custom types', sub { ); _ - is_deeply $txn->q(q{SELECT '{"(\"(2,{},bb)\",)","(\"(,,)\",bb)"}'::fupg_test_table[]})->val, [ + $val = $txn->q(q{SELECT '{"(\"(2,{},bb)\",)","(\"(,,)\",bb)"}'::fupg_test_table[]})->val; + is_deeply $val, [ { rec => { a => 2, aenum => [], domain => 'bb' }, dom => undef }, { rec => { a => undef, aenum => undef, domain => undef }, dom => 'bb' }, ]; + $val->[0] = 0; + $val->[1]{rec}{a} = 0; + $val->[1]{rec} = 0; + $val->[1]{dom} = 0; is $txn->q('SELECT $1::fupg_test_table[]', [ { rec => { a => 2, aenum => [], domain => 'bb' }, dom => undef }, diff --git a/t/pgtypes.t b/t/pgtypes.t index 5a17a71..3a3252c 100644 --- a/t/pgtypes.t +++ b/t/pgtypes.t @@ -21,10 +21,12 @@ sub v($type, $p_in, @args) { my $test = "$type $s_in" =~ s/\n/\\n/rg; utf8::encode($test); { - my $res = $conn->q("SELECT \$1::$type", $s_in)->text_params->val; + my $array = $conn->q("SELECT \$1::$type", $s_in)->text_params->flat; + my $res = $array->[0]; ok is_bool($res), "$test is bool" if $type eq 'bool'; ok created_as_number($res), "$test is number" if $type =~ /^(int|float)\d/; is_deeply $res, $p_out, "$test text->bin"; + $array->[0] = 0; # Must be writable } { my $res = $conn->q("SELECT \$1::$type", $p_in)->text_results->val; @@ -41,7 +43,11 @@ sub f($type, $p_in) { ok !eval { $conn->q("SELECT \$1::$type", $p_in)->val; 1 }, "$test fail"; } -ok !defined $conn->q('SELECT pg_sleep(0)')->val; # void +{ # void + my $array = $conn->q('SELECT pg_sleep(0)')->flat; + ok !defined $array->[0]; + $array->[0] = 0; +} v bool => true, undef, 1, 't'; v bool => false, undef, 0, 'f'; @@ -166,4 +172,10 @@ is $conn->q('SELECT ($1::int2[])[2]', [1,2,3,4])->val, 2; is $conn->q('SELECT ($1::int2vector)[1]', [1,2,3,4])->val, 2; is $conn->q('SELECT ($1::oidvector)[1]', [1,2,3,4])->val, 2; +{ + my $v = $conn->q("SELECT '{t,f,NULL}'::bool[]")->val; + is_deeply $v, [true, false, undef]; + $_ = 0 for @$v; +} + done_testing;