Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diesel_derives only considers flags comming from one of multiple #[diesel(…)] atttributes #2584

Closed
dawid-nowak opened this issue Nov 29, 2020 · 8 comments · Fixed by #2626

Comments

@dawid-nowak
Copy link

Is it supported at the moment to have 'serialize_as' and 'deserialize_as' on the single field in the sturcture as shown below?

#[derive(Identifiable, Queryable, PartialEq, Debug, Serialize, Deserialize, Insertable)]
#[table_name = "blah"]
pub struct Data {

    #[diesel(serialize_as = "SensitiveStringSerializer")]   
    #[diesel(deserialize_as = "SensitiveStringDeserializer")]

    pub sensitive_data: String
}
@weiznich
Copy link
Member

As the documentation mentions here deserialize_as is supported for the Queryable derive. serialize_as is not supported on any release yet.

Closed as this is not an issue or feature request. Use our gitter channel or our forum to ask for support or tho get answers to your questions.

@dawid-nowak
Copy link
Author

@weiznich
I do appreciate the prompt reply. The '''serialize_as''' is on the master branch. The issue is that only one of these annotations seems to be working at a time. I can either '''serialize_as''' or '''desrilalize_as''' but not both. In my opinion, that would be a bug, but I thought it might have been nice to doublecheck.

@weiznich
Copy link
Member

weiznich commented Dec 1, 2020

@dawid-nowak I that case I would appreciate getting a complete self contained example where you think something does not work as expected including the exact version you've used. Otherwise I do not see how this would be actionable at all.

@dawid-nowak
Copy link
Author

@weiznich
i hope this helps

#[macro_use]
extern crate diesel;
extern crate dotenv;
extern crate log;
mod schema;
use serde::{Deserialize, Serialize};
use log::info;
use std::io::Write;
use diesel::prelude::*;
use dotenv::dotenv;
use schema::*;
use std::env;
use diesel::{
    backend::Backend,
    deserialize::{
	Queryable,
	FromSql,
	FromSqlRow
    },
    expression::AsExpression,
    serialize::{ToSql, Output, self},
    sql_types,
};

#[derive(Debug, FromSqlRow, AsExpression)]
#[sql_type = "sql_types::Text"]
pub struct SensitiveStringSerializer(pub String);

impl Into<SensitiveStringSerializer> for String {
    fn into(self) -> SensitiveStringSerializer {
        SensitiveStringSerializer(self)
    }
}

impl<DB> ToSql<sql_types::Text, DB> for SensitiveStringSerializer
    where
        DB: Backend,
        String: ToSql<sql_types::Text, DB>,
{
    fn to_sql<W: Write>(&self, out: &mut Output<W, DB>) -> serialize::Result {
	let mut s = self.0.clone();
	s.replace_range(2..s.len()-2,"xxxxx");
 	info!("Writing Sensitive string {}",s);
        s.to_sql(out)
    }
}


#[derive(Debug,  AsExpression)]
pub struct SensitiveStringDeserializer(String);

impl<DB, ST> Queryable<ST, DB> for SensitiveStringDeserializer
where
    DB: Backend,
    String: Queryable<ST, DB>,
{
    type Row = <String as Queryable<ST, DB>>::Row;

    fn build(row: Self::Row) -> Self {
	let mut s = String::build(row);
	s.replace_range(2..s.len()-2,"ooooo");
 	info!("Reading Sensitive string {}",s);
        SensitiveStringDeserializer(s)
    }
}


impl Into<String> for SensitiveStringDeserializer {
    fn into(self) -> String {
        self.0
    }
}

#[derive(Identifiable, Queryable, PartialEq, Debug, Serialize, Deserialize, Insertable)]
#[table_name = "users"]
pub struct User {
    pub id: i32,
    
    #[diesel(serialize_as = "SensitiveStringSerializer")]   
    #[diesel(deserialize_as = "SensitiveStringDeserializer")]
    pub s1: String,

    #[diesel(deserialize_as = "SensitiveStringDeserializer")]    
    #[diesel(serialize_as = "SensitiveStringSerializer")]    
    pub s2: String,

    #[diesel(serialize_as = "SensitiveStringSerializer")]    
    pub s3: String,
    
    #[diesel(deserialize_as = "SensitiveStringDeserializer")]        
    pub s4: String,    
}

fn main() {
    env_logger::init();    
    dotenv().ok();
    let database_url = env::var("DATABASE_URL").expect("DATABASE_URL must be set");
    let conn = PgConnection::establish(&database_url).unwrap_or_else(|_| panic!("Error connecting to {}", database_url));
    let id = 7;
    let u1 = User{
	id,
	s1: "11111111111111111111".to_string(),
	s2: "22222222222222222222".to_string(),
	s3: "33333333333333333333".to_string(),
	s4: "44444444444444444444".to_string(),	
    };
    info!("u1 {:?}",u1);
    diesel::insert_into(schema::users::table).values(u1).execute(&conn).unwrap();
    info!("insert done");
    let u2 = schema::users::dsl::users.find(id).get_result::<User>(&conn).unwrap();
    info!("u2 {:?}",u2);       
}

expected is that User(s1:11111111, s2:22222222, s3:3333333,s4:444444444) will be written to database as User(s1:11xxxx11,s2:22xxxxxx22, s3: 33xxxxxx33, s4: 44444444).

expected is that on read User should be as follows User(s1:11oooooo11,s2:22ooooo22, s3: 33xxxxxxx33, s4: 44ooooooo44).

database view snapshot:
image

schema:
up.sql

CREATE TABLE users (
  id integer PRIMARY KEY,
  s1 VARCHAR NOT NULL,
  s2 VARCHAR NOT NULL,
  s3 VARCHAR NOT NULL,
  s4 VARCHAR NOT NULL
  
);

Cargo.toml

[package]
name = "diesel-problem"
version = "0.1.0"
edition = "2018"

[dependencies]	
diesel = { git = "https://github.com/diesel-rs/diesel", branch = "master", features = ["postgres"] }
dotenv = "0.15.0"
serde = { version = "1.0", features = ["derive"] }
serde_derive = "1.0"
serde_json = "1.0"
log = "0.4"
env_logger="0.8"

exact version
diesel v2.0.0 (https://github.com/diesel-rs/diesel?branch=master#daf1d331)

@weiznich
Copy link
Member

weiznich commented Dec 1, 2020

So I had a short look at this. Seems like we don't handle the case that some attribute occurs more than once well. If you change your code to the following everything will work fine:

#[derive(Identifiable, Queryable, PartialEq, Debug, Serialize, Deserialize, Insertable)]
#[table_name = "users"]
pub struct User {
    pub id: i32,
    
    #[diesel(serialize_as = "SensitiveStringSerializer", deserialize_as = "SensitiveStringDeserializer")]   
    pub s1: String,

    #[diesel(deserialize_as = "SensitiveStringDeserializer", serialize_as = "SensitiveStringSerializer")]    
    pub s2: String,

    #[diesel(serialize_as = "SensitiveStringSerializer")]    
    pub s3: String,
    
    #[diesel(deserialize_as = "SensitiveStringDeserializer")]        
    pub s4: String,    
}

(Note that I've changed just fields 1 and 2 to have the annotations in a combined attribute instead of having separate ones.)

Having said that: This is definitively something I would consider as bug, therefore I will reopen the issue. For the next time: Just fill out the issue template as it asks for most of the information required to reproduce nearly all bugs.

For anyone willing to look into fixing this: We would need change this function to not only return the first attribute with matching name, but all. That implies that we need to find a way to merge flags provided by multiple attributes + decide how to handle duplicated flags there.

@weiznich weiznich reopened this Dec 1, 2020
@weiznich weiznich changed the title Custom serialization/deserialization for a field Diesel_derives only considers flags comming from one of multiple #[diesel(…)] atttributes Dec 1, 2020
@dawid-nowak
Copy link
Author

now we are talking :) cheers

@pksunkara
Copy link
Member

I actually had this fixed in my branch of moving all attributes to diesel. I just need to finish it.

@pksunkara pksunkara self-assigned this Jan 6, 2021
@pksunkara
Copy link
Member

pksunkara commented Jan 14, 2021

That branch is blocked on #2557 which I will finish off first.

@pksunkara pksunkara linked a pull request Jan 19, 2021 that will close this issue
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants