From 921191912c48a68db81c02c02f3bc22e291d918c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 5 Jan 2016 15:00:54 -0500 Subject: [PATCH] In opr_sanity regression test, check for unexpected uses of cstring. In light of commit ea0d494dae0d3d6f, it seems like a good idea to add a regression test that will complain about random functions taking or returning cstring. Only I/O support functions and encoding conversion functions should be declared that way. While at it, add some checks that encoding conversion functions are declared properly. Since pg_conversion isn't populated manually, it's not quite as necessary to check its contents as it is for catalogs like pg_proc; but one thing we definitely have not tested in the past is whether the identified conproc for a conversion actually does that conversion vs. some other one. --- src/test/regress/expected/opr_sanity.out | 84 +++++++++++++++++++++++- src/test/regress/sql/opr_sanity.sql | 73 +++++++++++++++++++- 2 files changed, 155 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index 7c93b3b4ca..616e67405e 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -1,7 +1,7 @@ -- -- OPR_SANITY -- Sanity checks for common errors in making operator/procedure system tables: --- pg_operator, pg_proc, pg_cast, pg_aggregate, pg_am, +-- pg_operator, pg_proc, pg_cast, pg_conversion, pg_aggregate, pg_am, -- pg_amop, pg_amproc, pg_opclass, pg_opfamily, pg_index. -- -- Every test failure in this file should be closely inspected. @@ -319,6 +319,44 @@ ORDER BY 2; 3836 | range_recv (12 rows) +-- Look for functions that accept cstring and are neither datatype input +-- functions nor encoding conversion functions. It's almost never a good +-- idea to use cstring input for a function meant to be called from SQL; +-- text should be used instead, because cstring lacks suitable casts. +-- As of 9.6 this query should find only cstring_out and cstring_send. +-- However, we must manually exclude shell_in, which might or might not be +-- rejected by the EXISTS clause depending on whether there are currently +-- any shell types. +SELECT p1.oid, p1.proname +FROM pg_proc as p1 +WHERE 'cstring'::regtype = ANY (p1.proargtypes) + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typinput = p1.oid) + AND NOT EXISTS(SELECT 1 FROM pg_conversion WHERE conproc = p1.oid) + AND p1.oid != 'shell_in(cstring)'::regprocedure +ORDER BY 1; + oid | proname +------+-------------- + 2293 | cstring_out + 2501 | cstring_send +(2 rows) + +-- Likewise, look for functions that return cstring and aren't datatype output +-- functions nor typmod output functions. +-- As of 9.6 this query should find only cstring_in and cstring_recv. +-- However, we must manually exclude shell_out. +SELECT p1.oid, p1.proname +FROM pg_proc as p1 +WHERE p1.prorettype = 'cstring'::regtype + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid) + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid) + AND p1.oid != 'shell_out(opaque)'::regprocedure +ORDER BY 1; + oid | proname +------+-------------- + 2292 | cstring_in + 2500 | cstring_recv +(2 rows) + -- Check for length inconsistencies between the various argument-info arrays. SELECT p1.oid, p1.proname FROM pg_proc as p1 @@ -781,6 +819,50 @@ WHERE c.castmethod = 'b' AND xml | character | 0 | a (7 rows) +-- **************** pg_conversion **************** +-- Look for illegal values in pg_conversion fields. +SELECT p1.oid, p1.conname +FROM pg_conversion as p1 +WHERE p1.conproc = 0 OR + pg_encoding_to_char(conforencoding) = '' OR + pg_encoding_to_char(contoencoding) = ''; + oid | conname +-----+--------- +(0 rows) + +-- Look for conprocs that don't have the expected signature. +SELECT p.oid, p.proname, c.oid, c.conname +FROM pg_proc p, pg_conversion c +WHERE p.oid = c.conproc AND + (p.prorettype != 'void'::regtype OR p.proretset OR + p.pronargs != 5 OR + p.proargtypes[0] != 'int4'::regtype OR + p.proargtypes[1] != 'int4'::regtype OR + p.proargtypes[2] != 'cstring'::regtype OR + p.proargtypes[3] != 'internal'::regtype OR + p.proargtypes[4] != 'int4'::regtype); + oid | proname | oid | conname +-----+---------+-----+--------- +(0 rows) + +-- Check for conprocs that don't perform the specific conversion that +-- pg_conversion alleges they do, by trying to invoke each conversion +-- on some simple ASCII data. (The conproc should throw an error if +-- it doesn't accept the encodings that are passed to it.) +-- Unfortunately, we can't test non-default conprocs this way, because +-- there is no way to ask convert() to invoke them, and we cannot call +-- them directly from SQL. But there are no non-default built-in +-- conversions anyway. +-- (Similarly, this doesn't cope with any search path issues.) +SELECT p1.oid, p1.conname +FROM pg_conversion as p1 +WHERE condefault AND + convert('ABC'::bytea, pg_encoding_to_char(conforencoding), + pg_encoding_to_char(contoencoding)) != 'ABC'; + oid | conname +-----+--------- +(0 rows) + -- **************** pg_operator **************** -- Look for illegal values in pg_operator fields. SELECT p1.oid, p1.oprname diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql index c9bdd77da1..d6aa2e8623 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -1,7 +1,7 @@ -- -- OPR_SANITY -- Sanity checks for common errors in making operator/procedure system tables: --- pg_operator, pg_proc, pg_cast, pg_aggregate, pg_am, +-- pg_operator, pg_proc, pg_cast, pg_conversion, pg_aggregate, pg_am, -- pg_amop, pg_amproc, pg_opclass, pg_opfamily, pg_index. -- -- Every test failure in this file should be closely inspected. @@ -250,6 +250,36 @@ WHERE p1.prorettype IN 'anyrange'::regtype = ANY (p1.proargtypes)) ORDER BY 2; +-- Look for functions that accept cstring and are neither datatype input +-- functions nor encoding conversion functions. It's almost never a good +-- idea to use cstring input for a function meant to be called from SQL; +-- text should be used instead, because cstring lacks suitable casts. +-- As of 9.6 this query should find only cstring_out and cstring_send. +-- However, we must manually exclude shell_in, which might or might not be +-- rejected by the EXISTS clause depending on whether there are currently +-- any shell types. + +SELECT p1.oid, p1.proname +FROM pg_proc as p1 +WHERE 'cstring'::regtype = ANY (p1.proargtypes) + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typinput = p1.oid) + AND NOT EXISTS(SELECT 1 FROM pg_conversion WHERE conproc = p1.oid) + AND p1.oid != 'shell_in(cstring)'::regprocedure +ORDER BY 1; + +-- Likewise, look for functions that return cstring and aren't datatype output +-- functions nor typmod output functions. +-- As of 9.6 this query should find only cstring_in and cstring_recv. +-- However, we must manually exclude shell_out. + +SELECT p1.oid, p1.proname +FROM pg_proc as p1 +WHERE p1.prorettype = 'cstring'::regtype + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid) + AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid) + AND p1.oid != 'shell_out(opaque)'::regprocedure +ORDER BY 1; + -- Check for length inconsistencies between the various argument-info arrays. SELECT p1.oid, p1.proname @@ -426,6 +456,47 @@ WHERE c.castmethod = 'b' AND k.castsource = c.casttarget AND k.casttarget = c.castsource); + +-- **************** pg_conversion **************** + +-- Look for illegal values in pg_conversion fields. + +SELECT p1.oid, p1.conname +FROM pg_conversion as p1 +WHERE p1.conproc = 0 OR + pg_encoding_to_char(conforencoding) = '' OR + pg_encoding_to_char(contoencoding) = ''; + +-- Look for conprocs that don't have the expected signature. + +SELECT p.oid, p.proname, c.oid, c.conname +FROM pg_proc p, pg_conversion c +WHERE p.oid = c.conproc AND + (p.prorettype != 'void'::regtype OR p.proretset OR + p.pronargs != 5 OR + p.proargtypes[0] != 'int4'::regtype OR + p.proargtypes[1] != 'int4'::regtype OR + p.proargtypes[2] != 'cstring'::regtype OR + p.proargtypes[3] != 'internal'::regtype OR + p.proargtypes[4] != 'int4'::regtype); + +-- Check for conprocs that don't perform the specific conversion that +-- pg_conversion alleges they do, by trying to invoke each conversion +-- on some simple ASCII data. (The conproc should throw an error if +-- it doesn't accept the encodings that are passed to it.) +-- Unfortunately, we can't test non-default conprocs this way, because +-- there is no way to ask convert() to invoke them, and we cannot call +-- them directly from SQL. But there are no non-default built-in +-- conversions anyway. +-- (Similarly, this doesn't cope with any search path issues.) + +SELECT p1.oid, p1.conname +FROM pg_conversion as p1 +WHERE condefault AND + convert('ABC'::bytea, pg_encoding_to_char(conforencoding), + pg_encoding_to_char(contoencoding)) != 'ABC'; + + -- **************** pg_operator **************** -- Look for illegal values in pg_operator fields.