to_string() overflow

[aldebaranzbradaradjan]

Hi, I use actix_web to write an api, when I try to call the to_string() method of an branca::errors::Error my worker get stack overflow.

What I have try :

#[derive(Debug, Display)]
pub enum ApiError {
    InternalError(String),
}

#[derive(Debug, Deserialize, Serialize)]
pub struct ErrorResponse {
    errors: Vec<String>,
}

impl ResponseError for ApiError {
    fn error_response(&self) -> HttpResponse {
        match self {
            _ => HttpResponse::InternalServerError().json::<ErrorResponse>(self.into())
        }
    }
}

impl From<&String> for ErrorResponse {
    fn from(error: &String) -> Self {
        ErrorResponse {
            errors: vec![error.into()],
        }
    }
}

impl From<Vec<String>> for ErrorResponse {
    fn from(errors: Vec<String>) -> Self {
        ErrorResponse { errors }
    }
}

impl From<&ApiError> for ErrorResponse {
    fn from(error: &ApiError) -> Self {
        ErrorResponse {
            errors: vec![error.to_string()],
        }
    }
}

impl From<DieselError> for ApiError {
    fn from(error: DieselError) -> ApiError {
        ApiError::InternalError(error.to_string())
    }
}

impl From<BrancaError> for ApiError {
    fn from(error: BrancaError) -> ApiError {
        ApiError::InternalError(error.to_string())
    }
}

If I use description it works :

impl From<BrancaError> for ApiError {
    fn from(error: BrancaError) -> ApiError {
        ApiError::InternalError(error.description().to_string())
    }
}

It's an error on my side ? I'm new to rust, still learning.
See my error :

[2020-11-12T21:59:57Z INFO  actix_server::builder] Starting "actix-web-service-127.0.0.1:8080" service on 127.0.0.1:8080
thread 'actix-rt:worker:0' has overflowed its stack
fatal runtime error: stack overflow
[1]    1143042 abort (core dumped)  cargo run --release

Edit :
If I change the implementation of display error I manage to get it working, I don't know if it's a good way to do that but it's working for me :

impl fmt::Display for Error {
    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
        match *self {
            Error::InvalidBase62Token => write!(fmt, "Base62 token is invalid."),
            Error::InvalidTokenVersion => write!(fmt, "Token version is invalid."),
            Error::BadNonceLength => write!(fmt, "Bad nonce length."),
            Error::BadKeyLength => write!(fmt, "Bad key length."),
            Error::ExpiredToken => write!(fmt, "This token has expired."),
            Error::DecryptFailed => write!(fmt, "Decryption failed."),
            Error::EncryptFailed => write!(fmt, "Encryption failed."),
        }
         // write!(fmt, "{}", self.to_string()) <== this call to self.to_string() seems to throw an stackoverflow
    }
}

I'm not sure of how to do a clean disply error impl but I have see something similar in the chrono crate :
https://github.com/chronotope/chrono/blob/main/src/format/mod.rs (line 358) :

impl fmt::Display for ParseError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self.0 {
            ParseErrorKind::OutOfRange => write!(f, "input is out of range"),
            ParseErrorKind::Impossible => write!(f, "no possible date and time matching input"),
            ParseErrorKind::NotEnough => write!(f, "input is not enough for unique date and time"),
            ParseErrorKind::Invalid => write!(f, "input contains invalid characters"),
            ParseErrorKind::TooShort => write!(f, "premature end of input"),
            ParseErrorKind::TooLong => write!(f, "trailing input"),
            ParseErrorKind::BadFormat => write!(f, "bad or unsupported format string"),
        }
    }
}

[brycx]

Nice find @aldebaranzbradaradjan!

The Display trait is implemented with to_string():

branca/src/errors.rs

Line 25 in 34758ab

write!(fmt, "{}", self.to_string())

but Error does not provide a std::string::ToString implementation (when implementing Display correctly, ToString is automatically implemented). The Display impl has to use the derived Debug impl, if not a custom formatting:

write!(fmt, "{:?}", self)

Display tries to implement ToString based on itself, use it via Display ({}) and seems to overflow due to this recursion.

[aldebaranzbradaradjan]

Hey ! I had already seen that the debug allowed me to bypass the pb on my side:

impl From <BrancaError> for ApiError {
     fn from (error: BrancaError) -> ApiError {
         ApiError::InternalError( format!("{:?}", error) )
     }
}

But it was not very clean on this side, so I didn't think to use it to implement Display ! It's cool thank you! I will see to modify or redo my PR!

[brycx]

Fixed in c0e872c.