Fix valgrind warnings related to varcharvarbinary function (#2231)

Currently, we have defined the following cast functions which call varcharvarbinary C function with only 1 argument. But, in varcharvarbinary function, we are reading two arguments which might lead to memory related issues.

sys.binarysysvarchar
sys.binaryvarchar
sys.rowversionsysvarchar
sys.rowversionvarchar

This change modifies the above casts to have 3 arguments, one for input, one for typmod (integer) and the other for specifying isExplicit (boolean)

This change also replaces varcharTypeName->typeOid with baseTypeId in tsql_coerce_string_literal_hook() function as we are not setting varcharTypeName->typeOid at all which causes failure while trying to fetch the PG view definition as shown below.

Ex :
-- T-SQL endpoint
1> create view babel_4475_v1 as select cast(cast('a' as binary(2)) as varchar(2))
2> go

-- PSQL endpoint
jdbc_testdb=# select pg_catalog.pg_get_viewdef(oid, true) from pg_class where relname = 'babel_4475_v1';
ERROR:  cache lookup failed for type 0
jdbc_testdb=#

Issues Resolved: BABEL-4475
Signed-off-by: Sai Rohan Basa <[email protected]>

contrib/babelfishpg_common/sql/binary.sql

@@ -91,21 +91,21 @@ CREATE CAST (sys.BPCHAR AS sys.BBF_BINARY)
 WITH FUNCTION sys.bpcharbinary (sys.BPCHAR, integer, boolean) AS ASSIGNMENT;
 
 
-CREATE OR REPLACE FUNCTION sys.binarysysvarchar(sys.BBF_BINARY)
+CREATE OR REPLACE FUNCTION sys.binarysysvarchar(sys.BBF_BINARY, integer, boolean)
 RETURNS sys.VARCHAR
 AS 'babelfishpg_common', 'varbinaryvarchar'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
 CREATE CAST (sys.BBF_BINARY AS sys.VARCHAR)
-WITH FUNCTION sys.binarysysvarchar (sys.BBF_BINARY) AS ASSIGNMENT;
+WITH FUNCTION sys.binarysysvarchar (sys.BBF_BINARY, integer, boolean) AS ASSIGNMENT;
 
-CREATE OR REPLACE FUNCTION sys.binaryvarchar(sys.BBF_BINARY)
+CREATE OR REPLACE FUNCTION sys.binaryvarchar(sys.BBF_BINARY, integer, boolean)
 RETURNS pg_catalog.VARCHAR
 AS 'babelfishpg_common', 'varbinaryvarchar'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
 CREATE CAST (sys.BBF_BINARY AS pg_catalog.VARCHAR)
-WITH FUNCTION sys.binaryvarchar (sys.BBF_BINARY) AS ASSIGNMENT;
+WITH FUNCTION sys.binaryvarchar(sys.BBF_BINARY, integer, boolean) AS ASSIGNMENT;
 
 CREATE OR REPLACE FUNCTION sys.int2binary(INT2, integer, boolean)
 RETURNS sys.BBF_BINARY

contrib/babelfishpg_common/sql/rowversion.sql

@@ -100,21 +100,21 @@ CREATE CAST (sys.BPCHAR AS sys.ROWVERSION)
 WITH FUNCTION sys.bpcharrowversion (sys.BPCHAR, integer, boolean) AS ASSIGNMENT;
 
 
-CREATE OR REPLACE FUNCTION sys.rowversionsysvarchar(sys.ROWVERSION)
+CREATE OR REPLACE FUNCTION sys.rowversionsysvarchar(sys.ROWVERSION, integer, boolean)
 RETURNS sys.VARCHAR
 AS 'babelfishpg_common', 'varbinaryvarchar'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
 CREATE CAST (sys.ROWVERSION AS sys.VARCHAR)
-WITH FUNCTION sys.rowversionsysvarchar (sys.ROWVERSION) AS ASSIGNMENT;
+WITH FUNCTION sys.rowversionsysvarchar(sys.ROWVERSION, integer, boolean) AS ASSIGNMENT;
 
-CREATE OR REPLACE FUNCTION sys.rowversionvarchar(sys.ROWVERSION)
+CREATE OR REPLACE FUNCTION sys.rowversionvarchar(sys.ROWVERSION, integer, boolean)
 RETURNS pg_catalog.VARCHAR
 AS 'babelfishpg_common', 'varbinaryvarchar'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
 CREATE CAST (sys.ROWVERSION AS pg_catalog.VARCHAR)
-WITH FUNCTION sys.rowversionvarchar (sys.ROWVERSION) AS ASSIGNMENT;
+WITH FUNCTION sys.rowversionvarchar (sys.ROWVERSION, integer, boolean) AS ASSIGNMENT;
 
 CREATE OR REPLACE FUNCTION sys.int2rowversion(INT2, integer, boolean)
 RETURNS sys.ROWVERSION

contrib/babelfishpg_common/sql/upgrades/babelfish_common_helper--3.3.0--3.4.0.sql

@@ -7,10 +7,189 @@
 
 SELECT set_config('search_path', 'sys, '||current_setting('search_path'), false);
 
+-- Drops an object if it does not have any dependent objects.
+-- Is a temporary procedure for use by the upgrade script. Will be dropped at the end of the upgrade.
+-- Please have this be one of the first statements executed in this upgrade script. 
+CREATE OR REPLACE PROCEDURE babelfish_drop_deprecated_object(object_type varchar, schema_name varchar, object_name varchar) AS
+$$
+DECLARE
+    error_msg text;
+    query1 text;
+    query2 text;
+BEGIN
+
+    query1 := pg_catalog.format('alter extension babelfishpg_common drop %s %s.%s', object_type, schema_name, object_name);
+    query2 := pg_catalog.format('drop %s %s.%s', object_type, schema_name, object_name);
+
+    execute query1;
+    execute query2;
+EXCEPTION
+    when object_not_in_prerequisite_state then --if 'alter extension' statement fails
+        GET STACKED DIAGNOSTICS error_msg = MESSAGE_TEXT;
+        raise warning '%', error_msg;
+    when dependent_objects_still_exist then --if 'drop view' statement fails
+        GET STACKED DIAGNOSTICS error_msg = MESSAGE_TEXT;
+        raise warning '%', error_msg;
+end
+$$
+LANGUAGE plpgsql;
+
 CREATE OR REPLACE FUNCTION sys.varbinarybinary (sys.BBF_VARBINARY, integer, boolean)
 RETURNS sys.BBF_BINARY
 AS 'babelfishpg_common', 'binary'
 LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
 
+-- (sys.BBF_BINARY AS sys.VARCHAR)
+DROP CAST IF EXISTS(sys.BBF_BINARY AS sys.VARCHAR);
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    ALTER FUNCTION sys.binarysysvarchar(sys.BBF_BINARY) RENAME TO binarysysvarchar_deprecated_3_4_0;
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+CREATE OR REPLACE FUNCTION sys.binarysysvarchar(sys.BBF_BINARY, integer, boolean)
+RETURNS sys.VARCHAR
+AS 'babelfishpg_common', 'varbinaryvarchar'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.BBF_BINARY AS sys.VARCHAR)
+WITH FUNCTION sys.binarysysvarchar (sys.BBF_BINARY, integer, boolean) AS ASSIGNMENT;
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    CALL sys.babelfish_drop_deprecated_object('function', 'sys', 'binarysysvarchar_deprecated_3_4_0');
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+-- (sys.BBF_BINARY AS pg_catalog.VARCHAR)
+DROP CAST IF EXISTS(sys.BBF_BINARY AS pg_catalog.VARCHAR);
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    ALTER FUNCTION sys.binaryvarchar(sys.BBF_BINARY) RENAME TO binaryvarchar_deprecated_3_4_0;
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+CREATE OR REPLACE FUNCTION sys.binaryvarchar(sys.BBF_BINARY, integer, boolean)
+RETURNS pg_catalog.VARCHAR
+AS 'babelfishpg_common', 'varbinaryvarchar'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.BBF_BINARY AS pg_catalog.VARCHAR)
+WITH FUNCTION sys.binaryvarchar (sys.BBF_BINARY, integer, boolean) AS ASSIGNMENT;
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    CALL sys.babelfish_drop_deprecated_object('function', 'sys', 'binaryvarchar_deprecated_3_4_0');
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+-- (sys.ROWVERSION AS sys.VARCHAR)
+DROP CAST IF EXISTS(sys.ROWVERSION AS sys.VARCHAR);
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    ALTER FUNCTION sys.rowversionsysvarchar(sys.ROWVERSION) RENAME TO rowversionsysvarchar_deprecated_3_4_0;
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+CREATE OR REPLACE FUNCTION sys.rowversionsysvarchar(sys.ROWVERSION, integer, boolean)
+RETURNS sys.VARCHAR
+AS 'babelfishpg_common', 'varbinaryvarchar'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.ROWVERSION AS sys.VARCHAR)
+WITH FUNCTION sys.rowversionsysvarchar (sys.ROWVERSION, integer, boolean) AS ASSIGNMENT;
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    CALL sys.babelfish_drop_deprecated_object('function', 'sys', 'rowversionsysvarchar_deprecated_3_4_0');
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+-- (sys.BBF_BINARY AS pg_catalog.VARCHAR)
+DROP CAST IF EXISTS(sys.ROWVERSION AS pg_catalog.VARCHAR);
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    ALTER FUNCTION sys.rowversionvarchar(sys.ROWVERSION) RENAME TO rowversionvarchar_deprecated_3_4_0;
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+CREATE OR REPLACE FUNCTION sys.rowversionvarchar(sys.ROWVERSION, integer, boolean)
+RETURNS pg_catalog.VARCHAR
+AS 'babelfishpg_common', 'varbinaryvarchar'
+LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
+
+CREATE CAST (sys.ROWVERSION AS pg_catalog.VARCHAR)
+WITH FUNCTION sys.rowversionvarchar (sys.ROWVERSION, integer, boolean) AS ASSIGNMENT;
+
+DO $$
+DECLARE
+    exception_message text;
+BEGIN
+    CALL sys.babelfish_drop_deprecated_object('function', 'sys', 'rowversionvarchar_deprecated_3_4_0');
+
+EXCEPTION WHEN OTHERS THEN
+    GET STACKED DIAGNOSTICS
+    exception_message = MESSAGE_TEXT;
+    RAISE WARNING '%', exception_message;
+END;
+$$;
+
+-- Drops the temporary procedure used by the upgrade script.
+-- Please have this be one of the last statements executed in this upgrade script.
+DROP PROCEDURE sys.babelfish_drop_deprecated_object(varchar, varchar, varchar);
+
 -- Reset search_path to not affect any subsequent scripts
 SELECT set_config('search_path', trim(leading 'sys, ' from current_setting('search_path')), false);

contrib/babelfishpg_common/src/varbinary.c

@@ -725,8 +725,12 @@ bpcharvarbinary(PG_FUNCTION_ARGS)
 	PG_RETURN_BYTEA_P(result);
 }
 
-
-
+/*
+ * This function is currently being called with 1 and 3 arguments,
+ * Currently, the third argument is not being parsed in this function, 
+ * Check for the number of args needs to be added if the third arg is 
+ * being parsed in future
+ */
 Datum
 varbinaryvarchar(PG_FUNCTION_ARGS)
 {
@@ -735,12 +739,23 @@ varbinaryvarchar(PG_FUNCTION_ARGS)
 	VarChar    *result;
 	char 	   *encoded_result;
 	size_t		len = VARSIZE_ANY_EXHDR(source);
-	int32		typmod = PG_GETARG_INT32(1);
-	int32		maxlen = typmod - VARHDRSZ;
+	int32		typmod = -1;
+	int32		maxlen = -1;
 	coll_info	collInfo;
 	int			encodedByteLen;
 	MemoryContext ccxt = CurrentMemoryContext;
 
+	/*
+	 * Check whether the typmod argument exists, so that we 
+	 * will not be reading any garbage values for typmod 
+	 * which might cause Invalid read such as BABEL-4475
+	 */
+	if (PG_NARGS() > 1)
+	{
+		typmod = PG_GETARG_INT32(1);
+		maxlen = typmod - VARHDRSZ;
+	}
+
 	/*
 	 * Allow trailing null bytes 
 	 * Its safe since multi byte UTF-8 does not contain 0x00 

contrib/babelfishpg_tsql/src/pltsql_coerce.c

@@ -1150,7 +1150,7 @@ tsql_coerce_string_literal_hook(ParseCallbackState *pcbstate, Oid targetTypeId,
 
 			typenameTypeIdAndMod(NULL, (const TypeName *)varcharTypeName, &baseTypeId, &baseTypeMod);
 
-			tempcon = makeConst(varcharTypeName->typeOid, -1,
+			tempcon = makeConst(baseTypeId, -1,
 								tsql_get_server_collation_oid_internal(false),
 								-1, PointerGetDatum(cstring_to_text(value)),
 								false, false);

test/JDBC/expected/babel-4475-vu-cleanup.out

@@ -0,0 +1,29 @@
+drop view babel_4475_v1
+go
+
+drop view babel_4475_v2
+go
+
+drop view babel_4475_v3
+go
+
+drop view babel_4475_v4
+go
+
+drop function babel_4475_f1
+go
+
+drop function babel_4475_f2
+go
+
+drop procedure babel_4475_p1
+go
+
+drop procedure babel_4475_p2
+go
+
+drop procedure babel_4475_p3
+go
+
+drop procedure babel_4475_p4
+go

test/JDBC/expected/babel-4475-vu-prepare.out

@@ -0,0 +1,41 @@
+create view babel_4475_v1 as select cast(cast('a' as binary(2)) as varchar(2))
+go
+
+create function babel_4475_f1() returns table 
+as return (
+    select cast(cast('a' as binary(2)) as varchar(2))
+)
+go
+
+create procedure babel_4475_p1 as select cast(cast('a' as binary(2)) as varchar(2))
+go
+
+create view babel_4475_v2 as select cast(cast('a' as binary(2)) as pg_catalog.varchar(2))
+go
+
+create function babel_4475_f2() returns table 
+as return (
+    select cast(cast('a' as binary(2)) as pg_catalog.varchar(2))
+)
+go
+
+create procedure babel_4475_p2 as select cast(cast('a' as binary(2)) as pg_catalog.varchar(2))
+go
+
+EXEC sp_babelfish_configure 'babelfishpg_tsql.escape_hatch_rowversion', 'ignore';
+go
+
+create view babel_4475_v3 as select cast(cast('ab' as rowversion) as varchar(2))
+go
+
+create procedure babel_4475_p3 as select cast(cast('ab' as rowversion) as varchar(2))
+go
+
+create view babel_4475_v4 as select cast(cast('ab' as rowversion) as pg_catalog.varchar(2))
+go
+
+create procedure babel_4475_p4 as select cast(cast('ab' as rowversion) as pg_catalog.varchar(2))
+go
+
+EXEC sp_babelfish_configure 'babelfishpg_tsql.escape_hatch_rowversion', 'strict';
+go