#[serde(borrow)] does not work with Option<Cow<'a, str>>

[sffc]

Minimal repro:

#[derive(Debug, PartialEq, serde::Serialize, serde::Deserialize)]
struct Demo<'s> {
    #[serde(borrow)]
    message: Option<Cow<'s, str>>,
}

#[test]
fn demo() {
    let data_orig = Demo {
        message: Some(Cow::Borrowed("Hello World")),
    };
    let bincode_buf = bincode::serialize(&data_orig).expect("serialize");
    let data_new: Demo = bincode::deserialize(&bincode_buf).expect("deserialize from buffer");
    assert_eq!(data_orig, data_new);
    assert!(matches!(data_new.message, Some(Cow::Borrowed(_))));
}

Expected behavior: the test should pass, because the string in the Cow should be borrowed from the Bincode buffer.

Actual behavior: the test fails on the assert!(matches!(...)) line, and passes if I change Cow::Borrowed to Cow::Owned.

I believe it is desired to pass the lifetime down through the Option, since Serde does this with raw string references, according to #1029.

Related: #723, #1497

CC @Manishearth

[sffc]

Please note that fixing this bug may be a breaking change unfortunately, because people may have written data structs such as the above with the assumption that they worked with borrowed data, but then used those structs in a situation where DeserializeOwned is required.

[Manishearth]

As a workaround I wonder if it's possible for users to write a custom deserialize_fn which behaves much like de::borrow_cow_str and have that behave correctly?

[sffc]

I proposed a fix for this bug in #2072 but the fix was declined.

[jonasbb]

It is possible to use serde_with v1.10.0 to borrow when the Cow type is nested. From the release notes:

use serde_with::{serde_as, BorrowCow};
use std::borrow::Cow;

#[serde_as]
#[derive(Deserialize, Serialize)]
struct Data<'a> {
    #[serde_as(as = "Option<[BorrowCow; 1]>")]
    nested: Option<[Cow<'a, str>; 1]>,
}

let mut d: Data<'static> = serde_json::from_str(r#"{"nested": ["foobar"]}"#)?;
let [cow] = d.nested.take().unwrap();
match cow {
    Cow::Borrowed(_) => println!("borrowed"),
    Cow::Owned(_) => println!("owned"),
};
// => Prints borrowed

[sffc]

If this is closed as "won't fix", I'd like to suggest that at least serde(borrow) should complain if it tags something it doesn't know how to borrow.

The problem of getting the desired functionality is one thing, but the footgun of thinking you're borrowing when you're actually not is much worse.

[sffc]

As @robertbastian found in unicode-org/icu4x#1556, #[serde(borrow)] works with Option<Cow<'a, T>> but not Option<Cow<'a, str>>. I still believe this is a major footgun that should be fixed in Serde itself, either by making it work as expected or by throwing some sort of error.

Do you think we should reopen this issue to focus on that aspect?

[clarfonthey]

I think that it's… kind of ridiculous to have to wrap things in serde_as to get the proper borrow behaviour. Like, if the issue is that it's a breaking change, then… bump the major version, and fix it. Or, if you're that concerned, make serde(borrow_except_it_actually_works_correctly) and have that match the intended behaviour.

Sorry for being kinda salty about this, but I don't think that retaining compatibility with broken behaviour is a reason to not implement correct behaviour in any capacity.