Provide a way to retrieve both the request parts and transformed body in a single extractor

[tony707]

• I have looked for existing issues (including closed) about this

Feature Request

Motivation

Hey there!

I'm currently trying to migrate from axum 0.5.x to 0.6.x and I'm facing a little wall regarding body extractors.

I used to setup my handlers with both http::request::Parts and Json extractors in axum 0.5.x. to be able to do further processing on the headers and extensions for example:

async fn handler(parts: http::request::Parts, Json(body): Json<MyBody>) -> impl IntoResponse
{
}

In axum 0.6.x we can now only use a single body extractor (http::request::Parts being one of them), and I totally agree with that design choice since it's more efficient by avoiding some clones.

From what I can see from the implementation the parts get consumed internally after a call to Request::into_body() when using the Json extractor but I don't seem to see any reason why it couldn't also return the http::request::Parts in some way ?

I've been trying to use the http::Request<B> extractor along with calling into_parts() but in that case I loose the benefit of using the existing Json (or any other) body extractor and have to reimplement it myself.

Proposal

Provide a way to retrieve both the extracted body as well as the request parts in a single extractor.

async fn handler(JsonWithParts(parts, body): JsonWithParts<MyBody>) -> impl IntoResponse
{
}

Alternatives

If you see any alternative way of doing this, I am all ears :D

Thanks !

[davidpdrsn]

Parts doesn't implement FromRequestParts because it contains Extensions which cannot be cloned. So we'd have to remove the extensions which would make subsequent Extension extractors fail.

I recommend you extract the parts you need (Version, Method, HeaderMap, etc) directly.

[davidpdrsn]

could you please elaborate a little bit more? Imagine we want to write a wrapper for a Json that also extracts something from parts

Sure!

struct Foo<T> {
    matched_path: MatchedPath,
    value: T,
}

#[axum::async_trait]
impl<S, B, T> FromRequest<S, B> for Foo<T>
where
    B: Send + 'static,
    S: Send + Sync,
    Json<T>: FromRequest<S, B>,
{
    type Rejection = Response;

    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
        let (mut parts, body) = req.into_parts();

        let matched_path = MatchedPath::from_request_parts(&mut parts, state)
            .await
            .map_err(IntoResponse::into_response)?;

        let req = Request::from_parts(parts, body);

        let Json(value) = Json::<T>::from_request(req, state)
            .await
            .map_err(IntoResponse::into_response)?;

        Ok(Foo {
            matched_path,
            value,
        })
    }
}

I'm not sure why Request fields are private, it makes a lot of sense to make them public, as they already efficiently public but with into_parts/from_parts extra steps.

Thats not quite true. All that Request promises is that into_parts consumes the request and produces a fully owned Parts. It doesn't say anything about how those Parts are constructed. That mean Request is free to change its internal representation if one day we find ways to optimize it. If the fields were public we'd have much less flexibility to change it.

[Pzixel]

But changing internal structure would break into_parts/from_parts anyway (like, if new fields appear they must be returned by into_parts). Also it seems like a huge API limitation for a merely theoretical future change. This implementation could be just:

#[axum::async_trait]
impl<S, B, T> FromRequest<S, B> for Foo<T>
where
    B: Send + 'static,
    S: Send + Sync,
    Json<T>: FromRequest<S, B>,
{
    type Rejection = Response;

    async fn from_request(req: Request<B>, state: &S) -> Result<Self, Self::Rejection> {
        let matched_path = MatchedPath::from_request_parts(req.parts_mut(), state) // or &mut req.parts
            .await
            .map_err(IntoResponse::into_response)?;
        let Json(value) = Json::<T>::from_request(req, state)
            .await
            .map_err(IntoResponse::into_response)?;

        Ok(Foo {
            matched_path,
            value,
        })
    }
}

This looks much nicer IMHO. I also know a person (not me luckily) who spent 3 hours trying to figure out how this should be done. I only came here because of him, to clarify if it all is intended and isn't going to change.

[davidpdrsn]

But changint into_parts/from_parts would be breaking anyway if anything in internal sturcture changes (like, new fields appear).

No. Adding new fields to Parts is not a breaking change because you can't exhaustively match on it, since it has a private field today https://docs.rs/http/latest/src/http/request.rs.html#168-185. So you have to include a wildcard pattern when matching.

Also it seems like a huge API limitation for a merely theoretical future change. This implementation could be just:

Having a Request::parts_mut method would force Request to store a Parts internally because &mut Parts would have to be borrowed from self. The method would have to be

fn parts_mut<'self>(&'self mut self) -> &'self mut Parts

This is not the case with into_parts since there are no references. This was discussed in hyperium/http#511 and decided not to support.

[Pzixel]

No. Adding new fields to Parts is not a breaking change because you can't exhaustively match on it, since it has a private field today https://docs.rs/http/latest/src/http/request.rs.html#168-185. So you have to include a wildcard pattern when matching.

I didn't say anything about adding fields to Parts, I was saying about adding fields to Request. Adding a 3rd field would require to add 3rd tuple item to the into_parts which also a breaking change. Adding/removing private fields to Parts won't break anything. Because it's not ovservable.

This is not the case with into_parts since there are no references. This was discussed in hyperium/http#511 and decided not to support.

This is quite sad to hear. Existing api is quite awkward to use. But I probably have information you didn't yet hear so this makes this resolved. I think..

[davidpdrsn]

Adding a 3rd field would require to add 3rd tuple item to the into_parts which also a breaking change

Any new fields on Request can just be added to Parts. The point is that the current API gives the maintainers freedom to change the internals. Request::parts_mut doesn't support that.

This is quite sad to hear. Existing api is quite awkward to use.

I think the API is very reasonable but it seems you have very strong opinions about this so let's just agree to disagree.

[normundsneimanis]

Just in case anyone is struggling with this on axum 0.7, here's modified accepted example for this version.

pub struct Foo<T> {
    matched_path: MatchedPath,
    value: T,
}

#[axum::async_trait]
impl<S, T> FromRequest<S> for Foo<T>
    where
        S: Send + Sync,
        Json<T>: FromRequest<S>
{
    type Rejection = Response;

    async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
        let (mut parts, body) = req.into_parts();

        let matched_path = MatchedPath::from_request_parts(&mut parts, state)
            .await
            .map_err(IntoResponse::into_response)?;

        let req = Request::from_parts(parts, body);

        let Json(value) = Json::<T>::from_request(req, state)
            .await
            .map_err(IntoResponse::into_response)?;

        Ok(Foo {
            matched_path,
            value,
        })
    }
}