PathDeserializer: use deserialize_str for deserialize_any

[nitn3lav]

PR Type

probably bug fix?

PR Checklist

• Tests for the changes have been added / updated.
• Documentation comments have been added / updated.
• A changelog entry has been made for the appropriate packages.
• Format code with the latest stable rustfmt.
• (Team) Label with affected crates and semver status.

Overview

There are cases in which one might want to use types using deserialize_any in a web::Path. This is especially useful for enums with fields or when the same type is also used for other things, e. g. persisting to a database.
Since URLs are usually represented as Strings, I believe trying to deserialize a String if deserialize_any is called to be a reasonable default.

Examples

In this example it is unclear to the Deserialize impl whether to deserialize a str or u32 and the decision is therefore left to the Deserializer.

use actix_web::{get, web::Path, App, HttpResponse, HttpServer};
use serde::{de::Visitor, Deserialize};
use std::io;

#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub enum MyThing {
    String(String),
    Int(u32),
    Other,
}

impl<'de> Deserialize<'de> for MyThing {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        struct Vis;
        impl<'de> Visitor<'de> for Vis {
            type Value = MyThing;

            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                write!(formatter, "my thing")
            }

            fn visit_u32<E>(self, v: u32) -> Result<Self::Value, E>
            where
                E: serde::de::Error,
            {
                Ok(MyThing::Int(v))
            }

            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
            where
                E: serde::de::Error,
            {
                Ok(match v.parse() {
                    Ok(v) => MyThing::Int(v),
                    Err(e) => match v {
                        "other" => MyThing::Other,
                        _ => MyThing::String(format!("'{v}' is not an int because {e}")),
                    },
                })
            }
        }

        deserializer.deserialize_any(Vis)
    }
}

#[actix_web::main]
async fn main() -> io::Result<()> {
    HttpServer::new(move || App::new().service(root))
        .bind("127.0.0.1:8080")?
        .run()
        .await
}

#[get("/{my_thing}")]
async fn root(my_thing: Path<MyThing>) -> HttpResponse {
    HttpResponse::Ok().body(format!("it works: {my_thing:?}"))
}

In the current version of actix-web this fails because deserialize_any is not supported:

# curl "http://localhost:8080/123"
unsupported type: any%

Because this PR uses deserialize_str for deserialize_any, this now works as expected:

# curl "http://localhost:8080/123"
it works: Path(Int(123))%

# curl "http://localhost:8080/12334435543"
it works: Path(String("'12334435543' is not an int because number too large to fit in target type"))%

# curl "http://localhost:8080/123abc"
it works: Path(String("'123abc' is not an int because invalid digit found in string"))%

# curl "http://localhost:8080/other"
it works: Path(Other)%

Another example for a commonly used type like this is bson::uuid::Uuid. MongoDB stores UUIDs in a map containing the binary data along with a binary subtype. However deserializing Strings or raw binary data is supported, too. Therefore deserialize_any is used to indicate that many formats are supported.

This PR makes it possible to use types like MyThing and bson::uuid::Uuid in a web::Path.

Closes #318

[robjtede]

Marking as semver-major for now until I dig into this change properly.

In the meantime, I'd definitely like to see a test case added that would fail on master and pass on this PR.

[nitn3lav]

This PR adds the option to deserialize types whose Deserialize implementation uses deserialize_any which previously failed with unsupported type: any. Every other deserialization that worked before will continue to work as it did before.
The error always arises when a route tries to deserialize a type that uses deserialize_any and is therefore independent of any HTTP request sent by a client. I believe it to be very unlikely that an application using actix-web relies on this error. I therefore think that this can be safely added in a non-breaking minor change.

[robjtede]

This is my current hold-up. Ideally, if the AnyEnumDerive enum had Int variant before String then it should be able to deserialize to Int here. It's good that we can support it with a custom deserialize but I feel it's more distruptive to change this behavior later if it can be done at all.

[JohnTitor]

Addressed in 9c453c0.
We now does best-effort™ support for numeric values.

[JohnTitor]

Thank you for the PR!