From 1b7f668c43fdd45aba6eccb9b1e65061edb783bd Mon Sep 17 00:00:00 2001 From: Charles Samborski Date: Sun, 1 May 2022 08:55:55 +0200 Subject: [PATCH] Fix panics on unknown Postgres type oid when decoding Postgres arrays and records do not fully support custom types. When encountering an unknown OID, they currently default to using `PgTypeInfo::with_oid`. This is invalid as it breaks the invariant that decoding only uses resolved types, leading to panics. This commit returns an error instead of panicking. This is merely a mitigation: a proper fix would actually add full support for custom Postgres types. Full support involves more work, so it may still be useful to fix this immediate issue. Related issues: - https://github.com/launchbadge/sqlx/issues/1672 - https://github.com/launchbadge/sqlx/issues/1797 --- sqlx-core/src/postgres/types/array.rs | 7 +- sqlx-core/src/postgres/types/record.rs | 7 +- sqlx-core/src/query_builder.rs | 3 +- tests/postgres/postgres.rs | 154 +++++++++++++++++++++++++ 4 files changed, 166 insertions(+), 5 deletions(-) diff --git a/sqlx-core/src/postgres/types/array.rs b/sqlx-core/src/postgres/types/array.rs index b7514e8418..761b955dd6 100644 --- a/sqlx-core/src/postgres/types/array.rs +++ b/sqlx-core/src/postgres/types/array.rs @@ -134,7 +134,12 @@ where let element_type_oid = Oid(buf.get_u32()); let element_type_info: PgTypeInfo = PgTypeInfo::try_from_oid(element_type_oid) .or_else(|| value.type_info.try_array_element().map(Cow::into_owned)) - .unwrap_or_else(|| PgTypeInfo(PgType::DeclareWithOid(element_type_oid))); + .ok_or_else(|| { + BoxDynError::from(format!( + "failed to resolve array element type for oid {}", + element_type_oid.0 + )) + })?; // length of the array axis let len = buf.get_i32(); diff --git a/sqlx-core/src/postgres/types/record.rs b/sqlx-core/src/postgres/types/record.rs index 92ee1fa471..352bdb72b3 100644 --- a/sqlx-core/src/postgres/types/record.rs +++ b/sqlx-core/src/postgres/types/record.rs @@ -125,8 +125,6 @@ impl<'r> PgRecordDecoder<'r> { } }; - self.ind += 1; - if let Some(ty) = &element_type_opt { if !ty.is_null() && !T::compatible(ty) { return Err(mismatched_types::(ty)); @@ -134,7 +132,10 @@ impl<'r> PgRecordDecoder<'r> { } let element_type = - element_type_opt.unwrap_or_else(|| PgTypeInfo::with_oid(element_type_oid)); + element_type_opt + .ok_or_else(|| BoxDynError::from(format!("custom types in records are not fully supported yet: failed to retrieve type info for field {} with type oid {}", self.ind, element_type_oid.0)))?; + + self.ind += 1; T::decode(PgValueRef::get(&mut self.buf, self.fmt, element_type)) } diff --git a/sqlx-core/src/query_builder.rs b/sqlx-core/src/query_builder.rs index a8d8a38d99..761903f0e8 100644 --- a/sqlx-core/src/query_builder.rs +++ b/sqlx-core/src/query_builder.rs @@ -224,7 +224,7 @@ where /// // e.g. collect it to a `Vec` first. /// b.push_bind(user.id) /// .push_bind(user.username) - /// .push_bind(user.email) + /// .push_bind(user.email) /// .push_bind(user.password); /// }); /// @@ -310,6 +310,7 @@ where /// A wrapper around `QueryBuilder` for creating comma(or other token)-separated lists. /// /// See [`QueryBuilder::separated()`] for details. +#[allow(explicit_outlives_requirements)] pub struct Separated<'qb, 'args: 'qb, DB, Sep> where DB: Database, diff --git a/tests/postgres/postgres.rs b/tests/postgres/postgres.rs index e702b29c72..c3c7afc567 100644 --- a/tests/postgres/postgres.rs +++ b/tests/postgres/postgres.rs @@ -5,6 +5,7 @@ use sqlx::postgres::{ PgPoolOptions, PgRow, PgSeverity, Postgres, }; use sqlx::{Column, Connection, Executor, Row, Statement, TypeInfo}; +use sqlx_core::error::{BoxDynError, Error}; use sqlx_test::{new, pool, setup_if_needed}; use std::env; use std::sync::Arc; @@ -1200,6 +1201,159 @@ VALUES Ok(()) } +#[sqlx_macros::test] +async fn it_resolves_custom_types_in_anonymous_records() -> anyhow::Result<()> { + // This request involves nested records and array types. + + // Only supported in Postgres 11+ + let mut conn = new::().await?; + if matches!(conn.server_version_num(), Some(version) if version < 110000) { + return Ok(()); + } + + // language=PostgreSQL + conn.execute( + r#" +DROP TABLE IF EXISTS repo_users; +DROP TABLE IF EXISTS repositories; +DROP TABLE IF EXISTS repo_memberships; +DROP TYPE IF EXISTS repo_member; + +CREATE TABLE repo_users ( + user_id INT4 NOT NULL, + username TEXT NOT NULL, + PRIMARY KEY (user_id) +); +CREATE TABLE repositories ( + repo_id INT4 NOT NULL, + repo_name TEXT NOT NULL, + PRIMARY KEY (repo_id) +); +CREATE TABLE repo_memberships ( + repo_id INT4 NOT NULL, + user_id INT4 NOT NULL, + permission TEXT NOT NULL, + PRIMARY KEY (repo_id, user_id) +); +CREATE TYPE repo_member AS ( + user_id INT4, + permission TEXT +); +INSERT INTO repo_users(user_id, username) +VALUES + (101, 'alice'), + (102, 'bob'), + (103, 'charlie'); +INSERT INTO repositories(repo_id, repo_name) +VALUES + (201, 'rust'), + (202, 'sqlx'), + (203, 'hello-world'); +INSERT INTO repo_memberships(repo_id, user_id, permission) +VALUES + (201, 101, 'admin'), + (201, 102, 'write'), + (201, 103, 'read'), + (202, 102, 'admin'); +"#, + ) + .await?; + + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + struct RepoMember { + user_id: i32, + permission: String, + } + + impl sqlx::Type for RepoMember { + fn type_info() -> sqlx::postgres::PgTypeInfo { + sqlx::postgres::PgTypeInfo::with_name("repo_member") + } + } + + impl<'r> sqlx::Decode<'r, Postgres> for RepoMember { + fn decode( + value: sqlx::postgres::PgValueRef<'r>, + ) -> Result> { + let mut decoder = sqlx::postgres::types::PgRecordDecoder::new(value)?; + let user_id = decoder.try_decode::()?; + let permission = decoder.try_decode::()?; + Ok(Self { + user_id, + permission, + }) + } + } + + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + struct RepoMemberArray(Vec); + + impl sqlx::Type for RepoMemberArray { + fn type_info() -> sqlx::postgres::PgTypeInfo { + // Array type name is the name of the element type prefixed with `_` + sqlx::postgres::PgTypeInfo::with_name("_repo_member") + } + } + + impl<'r> sqlx::Decode<'r, Postgres> for RepoMemberArray { + fn decode( + value: sqlx::postgres::PgValueRef<'r>, + ) -> Result> { + Ok(Self(Vec::::decode(value)?)) + } + } + + let mut conn = new::().await?; + + #[derive(Debug, sqlx::FromRow)] + struct Row { + count: i64, + items: Vec<(i32, String, RepoMemberArray)>, + } + // language=PostgreSQL + let row: Result = sqlx::query_as::<_, Row>( + r" + WITH + members_by_repo AS ( + SELECT repo_id, + ARRAY_AGG(ROW (user_id, permission)::repo_member) AS members + FROM repo_memberships + GROUP BY repo_id + ), + repos AS ( + SELECT repo_id, repo_name, COALESCE(members, '{}') AS members + FROM repositories + LEFT OUTER JOIN members_by_repo USING (repo_id) + ORDER BY repo_id + ), + repo_array AS ( + SELECT COALESCE(ARRAY_AGG(repos.*), '{}') AS items + FROM repos + ), + repo_count AS ( + SELECT COUNT(*) AS count + FROM repos + ) + SELECT count, items + FROM repo_count, repo_array + ; + ", + ) + .fetch_one(&mut conn) + .await; + + // This test currently tests mitigations for `#1672` (use regular errors + // instead of panics). Once we fully support custom types, it should be + // updated accordingly. + match row { + Ok(_) => panic!("full support for custom types is not implemented yet"), + Err(e) => assert!(e + .to_string() + .contains("custom types in records are not fully supported yet")), + } + Ok(()) +} + #[sqlx_macros::test] async fn test_pg_server_num() -> anyhow::Result<()> { let conn = new::().await?;