Fix crash in json{b}_populate_recordset() and json{b}_to_recordset().

As of commit 37a795a60, populate_recordset_worker() tried to pass back
(as rsi.setDesc) a tupdesc that it also had cached in its fn_extra.
But the core executor would free the passed-back tupdesc, risking a
crash if the function were called again in the same query.  The safest
and least invasive way to fix that is to make an extra tupdesc copy
to pass back.

While at it, I failed to resist the temptation to get rid of unnecessary
get_fn_expr_argtype() calls here and in populate_record_worker().

Per report from Dmitry Dolgov; thanks to Michael Paquier and
Andrew Gierth for investigation and discussion.

Discussion: https://postgr.es/m/CA+q6zcWzN9ztCfR47ZwgTr1KLnuO6BAY6FurxXhovP4hxr+yOQ@mail.gmail.com
This commit is contained in:
Tom Lane 2018-07-13 14:16:47 -04:00
parent 2af39c39df
commit 9a5e8ed283
5 changed files with 59 additions and 23 deletions

View File

@ -421,9 +421,9 @@ static void sn_scalar(void *state, char *token, JsonTokenType tokentype);
/* worker functions for populate_record, to_record, populate_recordset and to_recordset */
static Datum populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
bool have_record_arg);
bool is_json, bool have_record_arg);
static Datum populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
bool have_record_arg);
bool is_json, bool have_record_arg);
/* helper functions for populate_record[set] */
static HeapTupleHeader populate_record(TupleDesc tupdesc, RecordIOData **record_p,
@ -2296,25 +2296,29 @@ elements_scalar(void *state, char *token, JsonTokenType tokentype)
Datum
jsonb_populate_record(PG_FUNCTION_ARGS)
{
return populate_record_worker(fcinfo, "jsonb_populate_record", true);
return populate_record_worker(fcinfo, "jsonb_populate_record",
false, true);
}
Datum
jsonb_to_record(PG_FUNCTION_ARGS)
{
return populate_record_worker(fcinfo, "jsonb_to_record", false);
return populate_record_worker(fcinfo, "jsonb_to_record",
false, false);
}
Datum
json_populate_record(PG_FUNCTION_ARGS)
{
return populate_record_worker(fcinfo, "json_populate_record", true);
return populate_record_worker(fcinfo, "json_populate_record",
true, true);
}
Datum
json_to_record(PG_FUNCTION_ARGS)
{
return populate_record_worker(fcinfo, "json_to_record", false);
return populate_record_worker(fcinfo, "json_to_record",
true, false);
}
/* helper function for diagnostics */
@ -3203,12 +3207,15 @@ populate_record(TupleDesc tupdesc,
return res->t_data;
}
/*
* common worker for json{b}_populate_record() and json{b}_to_record()
* is_json and have_record_arg identify the specific function
*/
static Datum
populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
bool have_record_arg)
bool is_json, bool have_record_arg)
{
int json_arg_num = have_record_arg ? 1 : 0;
Oid jtype = get_fn_expr_argtype(fcinfo->flinfo, json_arg_num);
JsValue jsv = {0};
HeapTupleHeader rec;
Datum rettuple;
@ -3216,8 +3223,6 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
MemoryContext fnmcxt = fcinfo->flinfo->fn_mcxt;
PopulateRecordCache *cache = fcinfo->flinfo->fn_extra;
Assert(jtype == JSONOID || jtype == JSONBOID);
/*
* If first time through, identify input/result record type. Note that
* this stanza looks only at fcinfo context, which can't change during the
@ -3303,9 +3308,9 @@ populate_record_worker(FunctionCallInfo fcinfo, const char *funcname,
PG_RETURN_NULL();
}
jsv.is_json = jtype == JSONOID;
jsv.is_json = is_json;
if (jsv.is_json)
if (is_json)
{
text *json = PG_GETARG_TEXT_PP(json_arg_num);
@ -3489,25 +3494,29 @@ hash_scalar(void *state, char *token, JsonTokenType tokentype)
Datum
jsonb_populate_recordset(PG_FUNCTION_ARGS)
{
return populate_recordset_worker(fcinfo, "jsonb_populate_recordset", true);
return populate_recordset_worker(fcinfo, "jsonb_populate_recordset",
false, true);
}
Datum
jsonb_to_recordset(PG_FUNCTION_ARGS)
{
return populate_recordset_worker(fcinfo, "jsonb_to_recordset", false);
return populate_recordset_worker(fcinfo, "jsonb_to_recordset",
false, false);
}
Datum
json_populate_recordset(PG_FUNCTION_ARGS)
{
return populate_recordset_worker(fcinfo, "json_populate_recordset", true);
return populate_recordset_worker(fcinfo, "json_populate_recordset",
true, true);
}
Datum
json_to_recordset(PG_FUNCTION_ARGS)
{
return populate_recordset_worker(fcinfo, "json_to_recordset", false);
return populate_recordset_worker(fcinfo, "json_to_recordset",
true, false);
}
static void
@ -3544,14 +3553,14 @@ populate_recordset_record(PopulateRecordsetState *state, JsObject *obj)
}
/*
* common worker for json_populate_recordset() and json_to_recordset()
* common worker for json{b}_populate_recordset() and json{b}_to_recordset()
* is_json and have_record_arg identify the specific function
*/
static Datum
populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
bool have_record_arg)
bool is_json, bool have_record_arg)
{
int json_arg_num = have_record_arg ? 1 : 0;
Oid jtype = get_fn_expr_argtype(fcinfo->flinfo, json_arg_num);
ReturnSetInfo *rsi;
MemoryContext old_cxt;
HeapTupleHeader rec;
@ -3662,7 +3671,7 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
state->cache = cache;
state->rec = rec;
if (jtype == JSONOID)
if (is_json)
{
text *json = PG_GETARG_TEXT_PP(json_arg_num);
JsonLexContext *lex;
@ -3693,8 +3702,6 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
bool skipNested = false;
JsonbIteratorToken r;
Assert(jtype == JSONBOID);
if (JB_ROOT_IS_SCALAR(jb) || !JB_ROOT_IS_ARRAY(jb))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@ -3726,8 +3733,13 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
}
}
/*
* Note: we must copy the cached tupdesc because the executor will free
* the passed-back setDesc, but we want to hang onto the cache in case
* we're called again in the same query.
*/
rsi->setResult = state->tuple_store;
rsi->setDesc = cache->c.io.composite.tupdesc;
rsi->setDesc = CreateTupleDescCopy(cache->c.io.composite.tupdesc);
PG_RETURN_NULL();
}

View File

@ -1841,6 +1841,16 @@ SELECT json_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
(0,1)
(1 row)
SELECT i, json_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
FROM (VALUES (1),(2)) v(i);
i | json_populate_recordset
---+-------------------------
1 | (42,50)
1 | (1,43)
2 | (42,50)
2 | (2,43)
(4 rows)
-- composite domain
SELECT json_populate_recordset(null::j_ordered_pair, '[{"x": 0, "y": 1}]');
json_populate_recordset

View File

@ -2523,6 +2523,16 @@ SELECT jsonb_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
(0,1)
(1 row)
SELECT i, jsonb_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
FROM (VALUES (1),(2)) v(i);
i | jsonb_populate_recordset
---+--------------------------
1 | (42,50)
1 | (1,43)
2 | (42,50)
2 | (2,43)
(4 rows)
-- composite domain
SELECT jsonb_populate_recordset(null::jb_ordered_pair, '[{"x": 0, "y": 1}]');
jsonb_populate_recordset

View File

@ -547,6 +547,8 @@ select * from json_populate_recordset(row('def',99,null)::jpop,'[{"a":[100,200,3
-- anonymous record type
SELECT json_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
SELECT json_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
SELECT i, json_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
FROM (VALUES (1),(2)) v(i);
-- composite domain
SELECT json_populate_recordset(null::j_ordered_pair, '[{"x": 0, "y": 1}]');

View File

@ -663,6 +663,8 @@ SELECT * FROM jsonb_populate_recordset(row('def',99,NULL)::jbpop,'[{"a":[100,200
-- anonymous record type
SELECT jsonb_populate_recordset(null::record, '[{"x": 0, "y": 1}]');
SELECT jsonb_populate_recordset(row(1,2), '[{"f1": 0, "f2": 1}]');
SELECT i, jsonb_populate_recordset(row(i,50), '[{"f1":"42"},{"f2":"43"}]')
FROM (VALUES (1),(2)) v(i);
-- composite domain
SELECT jsonb_populate_recordset(null::jb_ordered_pair, '[{"x": 0, "y": 1}]');