From 7d3b2050c99ee1553329d1b83575190ea955a576 Mon Sep 17 00:00:00 2001 From: Oliver Rice Date: Wed, 3 Jan 2024 15:47:11 -0600 Subject: [PATCH] plug type system hole where functions returning views can cause invalid refs --- src/graphql.rs | 6 +- test/expected/function_calls.out | 6 +- .../function_return_view_has_pkey.out | 122 ++++++++++++++++++ test/sql/function_calls.sql | 26 +--- test/sql/function_return_view_has_pkey.sql | 71 ++++++++++ 5 files changed, 203 insertions(+), 28 deletions(-) create mode 100644 test/expected/function_return_view_has_pkey.out create mode 100644 test/sql/function_return_view_has_pkey.sql diff --git a/src/graphql.rs b/src/graphql.rs index 4c24db4f..cac4e918 100644 --- a/src/graphql.rs +++ b/src/graphql.rs @@ -1289,9 +1289,11 @@ fn function_fields(schema: &Arc<__Schema>, volatilities: &[FunctionVolatility]) // If the return type is a table type, it must be selectable if !match &return_type { - __Type::Node(table_type) => table_type.table.permissions.is_selectable, + __Type::Node(table_type) => { + schema.graphql_table_select_types_are_valid(&table_type.table) + } __Type::Connection(table_type) => { - table_type.table.permissions.is_selectable + schema.graphql_table_select_types_are_valid(&table_type.table) } _ => true, } { diff --git a/test/expected/function_calls.out b/test/expected/function_calls.out index a90e1220..c6c895b2 100644 --- a/test/expected/function_calls.out +++ b/test/expected/function_calls.out @@ -2601,7 +2601,7 @@ begin; rollback to savepoint a; create table account( - id int, + id int primary key, email varchar(255), name text null ); @@ -2620,8 +2620,8 @@ begin; insert into account(id, email, name) values (1, 'aardvark@x.com', 'aardvark'),--all columns non-null - (2, 'bat@x.com', null),--mixed: some null, some non-null - (null, null, null);--all columns null + (2, 'bat@x.com', null);--mixed: some null, some non-null + --(null, null, null);--all columns null -- comment on table account is e'@graphql({"totalCount": {"enabled": true}})'; select jsonb_pretty(graphql.resolve($$ query { diff --git a/test/expected/function_return_view_has_pkey.out b/test/expected/function_return_view_has_pkey.out new file mode 100644 index 00000000..a3283c78 --- /dev/null +++ b/test/expected/function_return_view_has_pkey.out @@ -0,0 +1,122 @@ +begin; + create view account as + select + 1 as foo, + 2 as bar; + create function returns_account() + returns account language sql stable + as $$ select foo, bar from account; $$; + -- Account should not be visible because the view has no primary key + select jsonb_pretty( + graphql.resolve($$ + { + __type(name: "Account") { + __typename + } + } + $$) + ); + jsonb_pretty +------------------------ + { + + "data": { + + "__type": null+ + } + + } +(1 row) + + -- returnsAccount should also not be visible because account has no primary key + select jsonb_pretty( + graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + } + } + } + } + $$) + ); + jsonb_pretty +---------------------------------------- + { + + "data": { + + "__schema": { + + "queryType": { + + "fields": [ + + { + + "name": "node"+ + } + + ] + + } + + } + + } + + } +(1 row) + + comment on view account is e' + @graphql({ + "primary_key_columns": ["foo"] + })'; + -- Account should be visible because the view is selectable and has a primary key + select jsonb_pretty( + graphql.resolve($$ + { + __type(name: "Account") { + __typename + } + } + $$) + ); + jsonb_pretty +------------------------------------- + { + + "data": { + + "__type": { + + "__typename": "Account"+ + } + + } + + } +(1 row) + + -- returnsAccount should also be visible because account has a primary key and is selectable + select jsonb_pretty( + graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + } + } + } + } + $$) + ); + jsonb_pretty +----------------------------------------------------- + { + + "data": { + + "__schema": { + + "queryType": { + + "fields": [ + + { + + "name": "accountCollection"+ + }, + + { + + "name": "node" + + }, + + { + + "name": "returnsAccount" + + } + + ] + + } + + } + + } + + } +(1 row) + + +rollback; diff --git a/test/sql/function_calls.sql b/test/sql/function_calls.sql index 99ae8aa5..d597211e 100644 --- a/test/sql/function_calls.sql +++ b/test/sql/function_calls.sql @@ -807,7 +807,7 @@ begin; rollback to savepoint a; create table account( - id int, + id int primary key, email varchar(255), name text null ); @@ -820,10 +820,6 @@ begin; returns account language sql stable as $$ select id, email, name from account where id = 2; $$; - create function returns_all_columns_null_account() - returns account language sql stable - as $$ select id, email, name from account where id is null; $$; - create function returns_null_account() returns account language sql stable as $$ select id, email, name from account where id = 9; $$; @@ -831,10 +827,8 @@ begin; insert into account(id, email, name) values (1, 'aardvark@x.com', 'aardvark'),--all columns non-null - (2, 'bat@x.com', null),--mixed: some null, some non-null - (null, null, null);--all columns null + (2, 'bat@x.com', null);--mixed: some null, some non-null - -- comment on table account is e'@graphql({"totalCount": {"enabled": true}})'; select jsonb_pretty(graphql.resolve($$ query { @@ -858,21 +852,7 @@ begin; } $$)); - -- With current implementation we can't distinguish between - -- when all columns of a composite type are null and when - -- the composite type itself is null. In both these cases - -- the result will be null for the top-level field. - select jsonb_pretty(graphql.resolve($$ - query { - returnsAllColumnsNullAccount { - id - email - name - __typename - } - } - $$)); - + -- When no record is found, the top level field becomes null select jsonb_pretty(graphql.resolve($$ query { returnsNullAccount { diff --git a/test/sql/function_return_view_has_pkey.sql b/test/sql/function_return_view_has_pkey.sql new file mode 100644 index 00000000..c129ac16 --- /dev/null +++ b/test/sql/function_return_view_has_pkey.sql @@ -0,0 +1,71 @@ +begin; + + create view account as + select + 1 as foo, + 2 as bar; + + create function returns_account() + returns account language sql stable + as $$ select foo, bar from account; $$; + + -- Account should not be visible because the view has no primary key + select jsonb_pretty( + graphql.resolve($$ + { + __type(name: "Account") { + __typename + } + } + $$) + ); + + -- returnsAccount should also not be visible because account has no primary key + select jsonb_pretty( + graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + } + } + } + } + $$) + ); + + comment on view account is e' + @graphql({ + "primary_key_columns": ["foo"] + })'; + + -- Account should be visible because the view is selectable and has a primary key + select jsonb_pretty( + graphql.resolve($$ + { + __type(name: "Account") { + __typename + } + } + $$) + ); + + -- returnsAccount should also be visible because account has a primary key and is selectable + select jsonb_pretty( + graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + } + } + } + } + $$) + ); + + + +rollback;