ureq icon indicating copy to clipboard operation
ureq copied to clipboard

Invalid transformation to http::Response<Vec<u8>> for non utf8 response bodies

Open rogercoll opened this issue 6 months ago • 3 comments

When using protobuffers over HTTP the library silently transforms the protobuf data into an invalid http::Response<Vec<u8>>. Decoding protobuf crates like Prost fail to decode the generated Vec<u8>.

It seems the issue is because the crate is trying to transform the body into a String and then to Vec: https://github.com/algesten/ureq/blob/main/src/http_interop.rs#L132

When dealing with binary data ("content-type": "application/x-protobuf"), it shouldn't be converted into a String. https://github.com/algesten/ureq/blob/main/src/http_interop.rs#L132

We have been able to fix the issue by manually converting the internal reader into a Vec.

What do you think of adding a content type check before transforming the Reader into a String?

We are able to contribute with the decided solution.

rogercoll avatar Jan 09 '24 17:01 rogercoll

Thanks for reporting this. Looking at that code, it's just wrong regardless of content type.

Instead of checking content type, that code should unconditionally call into_reader(), then read the result into a Vec<u8>.

jsha avatar Jan 09 '24 18:01 jsha

Make sense to me. Wdyt of https://github.com/algesten/ureq/pull/702?

rogercoll avatar Jan 10 '24 07:01 rogercoll

It would be great to have the happy path transformation too (TryFrom). Which would be the best way to define the possible error? (E.g io::Error)

Would it be possible to extend the ureq::Error with a new enum variant:

#[derive(Debug)]
pub enum Error {
    /// A response was successfully received but had status code >= 400.
    /// Values are (status_code, Response).
    Status(u16, Response),
    /// There was an error making the request or receiving the response.
    Transport(Transport),
     #[cfg(features = ...)]
     /// There was an error while transforming the request to http::Request.
    ConvertError(String),
}

rogercoll avatar Jan 10 '24 08:01 rogercoll