Merge pull request #569 from diesel-rs/sg-escape-identifier

Don't use libpq for identifier escaping

diesel/src/pg/connection/mod.rs

@@ -28,7 +28,7 @@ use types::HasSqlType;
 /// http://www.postgresql.org/docs/9.4/static/libpq-connect.html#LIBPQ-CONNSTRING
 #[allow(missing_debug_implementations)]
 pub struct PgConnection {
-    raw_connection: Rc<RawConnection>,
+    raw_connection: RawConnection,
     transaction_depth: Cell<i32>,
     statement_cache: StatementCache,
 }
@@ -52,7 +52,7 @@ impl Connection for PgConnection {
     fn establish(database_url: &str) -> ConnectionResult<PgConnection> {
         RawConnection::establish(database_url).map(|raw_conn| {
             PgConnection {
-                raw_connection: Rc::new(raw_conn),
+                raw_connection: raw_conn,
                 transaction_depth: Cell::new(0),
                 statement_cache: StatementCache::new(),
             }
@@ -154,7 +154,7 @@ impl PgConnection {
                 bind_types,
             ))
         } else {
-            let mut query_builder = PgQueryBuilder::new(&self.raw_connection);
+            let mut query_builder = PgQueryBuilder::new();
             try!(source.to_sql(&mut query_builder).map_err(Error::QueryBuilderError));
             Rc::new(try!(Query::sql(&query_builder.sql, Some(bind_types))))
         };

diesel/src/pg/connection/raw.rs

@@ -35,22 +35,6 @@ impl RawConnection {
         last_error_message(self.internal_connection)
     }
 
-    pub fn escape_identifier(&self, identifier: &str) -> Result<PgString, String> {
-        let result_ptr = unsafe { PQescapeIdentifier(
-            self.internal_connection,
-            identifier.as_ptr() as *const libc::c_char,
-            identifier.len() as libc::size_t,
-        ) };
-
-        if result_ptr.is_null() {
-            Err(last_error_message(self.internal_connection))
-        } else {
-            unsafe {
-                Ok(PgString::new(result_ptr))
-            }
-        }
-    }
-
     pub fn set_notice_processor(&self, notice_processor: NoticeProcessor) {
         unsafe {
             PQsetNoticeProcessor(self.internal_connection, Some(notice_processor), ptr::null_mut());
@@ -139,38 +123,6 @@ fn last_error_message(conn: *const PGconn) -> String {
     }
 }
 
-#[allow(missing_debug_implementations, missing_copy_implementations)]
-pub struct PgString {
-    pg_str: *mut libc::c_char,
-}
-
-impl PgString {
-    unsafe fn new(ptr: *mut libc::c_char) -> Self {
-        PgString {
-            pg_str: ptr,
-        }
-    }
-}
-
-impl ::std::ops::Deref for PgString {
-    type Target = str;
-
-    fn deref(&self) -> &str {
-        unsafe {
-            let c_string = CStr::from_ptr(self.pg_str);
-            str::from_utf8_unchecked(c_string.to_bytes())
-        }
-    }
-}
-
-impl Drop for PgString {
-    fn drop(&mut self) {
-        unsafe {
-            PQfreemem(self.pg_str as *mut libc::c_void)
-        }
-    }
-}
-
 /// Internal wrapper around a `*mut PGresult` which is known to be not-null, and
 /// have no aliases.  This wrapper is to ensure that it's always properly
 /// dropped.

diesel/src/pg/connection/stmt/cache.rs

@@ -49,7 +49,7 @@ impl StatementCache {
 
     pub fn cached_query<T: QueryFragment<Pg> + QueryId>(
         &self,
-        conn: &Rc<RawConnection>,
+        conn: &RawConnection,
         source: &T,
         bind_types: Vec<u32>,
     ) -> QueryResult<Rc<Query>> {
@@ -58,15 +58,15 @@ impl StatementCache {
 
         let cache_suffix = self.cache.borrow().len() + 1;
         let mut cache = self.cache.borrow_mut();
-        let (cache_key, maybe_binds) = try!(cache_key(conn, source, bind_types));
+        let (cache_key, maybe_binds) = try!(cache_key(source, bind_types));
 
         match cache.entry(cache_key) {
             Occupied(entry) => Ok(entry.get().clone()),
             Vacant(entry) => {
                 let statement = {
                     let sql = match entry.key().sql() {
                         Some(sql) => Cow::Borrowed(sql),
-                        None => Cow::Owned(try!(to_sql(conn, source))),
+                        None => Cow::Owned(try!(to_sql(source))),
                     };
 
                     let name = format!("__diesel_stmt_{}", cache_suffix);
@@ -104,24 +104,23 @@ impl StatementCache {
     }
 }
 
-fn to_sql<T: QueryFragment<Pg>>(conn: &Rc<RawConnection>, source: &T)
+fn to_sql<T: QueryFragment<Pg>>(source: &T)
     -> QueryResult<String>
 {
-    let mut query_builder = PgQueryBuilder::new(conn);
+    let mut query_builder = PgQueryBuilder::new();
     try!(source.to_sql(&mut query_builder).map_err(QueryBuilderError));
     Ok(query_builder.sql)
 }
 
 fn cache_key<T: QueryFragment<Pg> + QueryId>(
-    conn: &Rc<RawConnection>,
     source: &T,
     bind_types: Vec<u32>,
 ) -> QueryResult<(StatementCacheKey, Option<Vec<u32>>)> {
     match T::query_id() {
         Some(id) => Ok((StatementCacheKey::Type(id), Some(bind_types))),
         None => Ok((
             StatementCacheKey::Query {
-                sql: try!(to_sql(conn, source)),
+                sql: try!(to_sql(source)),
                 bind_types: bind_types
             },
             None,

diesel/src/pg/query_builder.rs

@@ -1,20 +1,15 @@
-use std::rc::Rc;
-
 use super::backend::Pg;
-use super::connection::raw::RawConnection;
 use query_builder::{QueryBuilder, BuildQueryResult};
 
 #[allow(missing_debug_implementations)]
 pub struct PgQueryBuilder {
-    conn: Rc<RawConnection>,
     pub sql: String,
     bind_idx: u32,
 }
 
 impl PgQueryBuilder {
-    pub fn new(conn: &Rc<RawConnection>) -> Self {
+    pub fn new() -> Self {
         PgQueryBuilder {
-            conn: conn.clone(),
             sql: String::new(),
             bind_idx: 0,
         }
@@ -27,8 +22,10 @@ impl QueryBuilder<Pg> for PgQueryBuilder {
     }
 
     fn push_identifier(&mut self, identifier: &str) -> BuildQueryResult {
-        let escaped_identifier = try!(self.conn.escape_identifier(identifier));
-        Ok(self.push_sql(&escaped_identifier))
+        self.push_sql("\"");
+        self.push_sql(&identifier.replace('"', "\"\""));
+        self.push_sql("\"");
+        Ok(())
     }
 
     fn push_bind_param(&mut self) {