seed icon indicating copy to clipboard operation
seed copied to clipboard

Checking content-type on a Response

Open techninja1008 opened this issue 5 years ago • 5 comments

I have a case where I need to check that the content-type of a response is application/json in order to ensure validity. My current method of doing this is to go via raw_response(), which remarks that I should create an issue to discuss my use case.

In my mind, the following additions to Response would cover my use case:

  • fn content_type(&self) -> fetch::Result<MIMEType>
  • fn check_content_type(&self, mime: MIMEType) -> fetch::Result<Self> (would be to content_type as check_status is to status)

One detail at the very least which would need discussion would be whether MIMEType should be an enum parsed from the String value or simply just a String.

techninja1008 avatar Sep 02 '20 10:09 techninja1008

content_type is a header so I would make it more general. The first idea:

  • fn headers(&self) -> &fetch::Headers
  • fn check_headers(&self, impl FnOnce(&fetch::Headers, &mut [String])) -> fetch::Result<Self>
    • It would return either Ok<Self> or FetchError::HeaderErrors<Vec<String>>.
    • Usage:
      .check_headers(|headers, errors|  {
          if let Some(content_type_header) = headers.get(Header::ContentType) {
               if MIMEType::ApplicationJson != content_type_header.value() {
                    errors.push(format!("Header content_type has invalid value: {}", content_type.value()));
               }
          } else {
               errors.push("Header content_type is missing.".to_owned());
          }
      })?
      

What do you think?

MartinKavik avatar Sep 02 '20 15:09 MartinKavik

I like it! How do you propose the content_type_header.value() bit to work? (Specifically conversion from a generic Header type to a type specific for the header)

techninja1008 avatar Sep 02 '20 16:09 techninja1008

How do you propose the content_type_header.value() bit to work?

It was just the first idea. I would need more time to think about it, but unfortunately it hasn't the highest priority now so I don't know when I will be able to design and implement it properly. However if you want to move it further you can write an API design proposal and I can give you feedback.

MartinKavik avatar Sep 02 '20 17:09 MartinKavik

Sure! After feedback, I'm also happy to implement and put in a PR if you want?

HeaderType enum

Fairly self-explanatory.

pub enum HeaderType {
    ContentType,

    // ... Possibly others? ...

    Custom(String),
}

ResponseHeaders struct

pub struct ResponseHeaders {
    // Stores a web_sys::Headers
}

// Would have an impl with the get function as shown above, returning a ReponseHeader

ResponseHeader

pub struct ResponseHeader {
    name: String,
    value: String,
}

impl ResponseHeader {
    fn value<T>(&self) -> Option<T> where T: FromHeader {
        T::from_header(self.name.as_str(), self.value.as_str())
    }
}

FromHeader trait

pub trait FromHeader {
    fn from_header(name: &str, value: &str) -> Option<Self>;
}

The idea is that MIMEType (and other header value types) would implement FromHeader, check that they're being used on the correct header(s) (if not, return None), parse, and return Some(parsed_value).

MIMEType

Perhaps use the mime crate? It seems to comprehensively support many mime types already with no deps.

Usage

On Response:

  • fn headers(&self) -> fetch::ResponseHeaders
  • fn check_header<T: FromHeader, E>(&self, HeaderType, impl FnOnce(Option<T>) -> Result<(), E>) -> Result<Self, E>
  • fn check_headers<E>(&self, impl FnOnce(&fetch::Headers) -> Result<(), E>) -> Result<Self, E>

Usage of a plain result here is so that the error can be passed through from the callback, giving the user the option of either returning a fetch error (making it a fetch::Result) or their own type. We'd still add FetchError::HeaderErrors<Vec<String>> for this use.

Examples:

response.check_header(HeaderType::ContentType, |maybe_ty: Option<Mime>| if Some(ty) = maybe_ty {
    if ty == mime::APPLICATION_JSON {
        Ok(())
    }else{
        Err(FetchError::HeaderErrors(vec!["Incorrect content type".into()]))
    }
}else{
    Err(FetchError::HeaderErrors(vec!["Missing content type".into()]))
})

techninja1008 avatar Sep 02 '20 19:09 techninja1008

If we gonna have multiple check_xxx methods, then perhaps

request
   .fetch()
   .await
   .and_then(Status::check) // fetch::Result<Response> -> fetch::Result<Response>
   .and_then(Headers::check_content_type(&["text/plain"])) // fetch::Result -> fetch::Result
   .and_then(|response| my_check_headers(response.headers())) // fetch::Result -> fetch::Result
   .unwrap() // fetch::Result -> Response

/// accept 2xx
fn Status::check(Response) -> fetch::Result<Response>
/// accept only provided mime types
fn Headers::check_content_type(&[Into<MimeType>]) -> ( Fn (Response) -> fetch::Result<Response> )

TatriX avatar Sep 07 '20 11:09 TatriX

obsolete since v0.10.0

flosse avatar Mar 07 '23 00:03 flosse