Support `.cookie(...)` for `customize()`

[kukuro-kt]

As https://github.com/actix/actix-web/discussions/3213 wrote, something like customize().cookie(...).

I'm not sure if I need to write more details?

[Thomblin]

I want to give this a try. See https://github.com/actix/actix-web/pull/3215 for my first draft

[kukuro-kt]

One more thing, why cookie needs to be converted to HeaderValue first, while append_header(...) can insert String directly?

Current implement of cookie(...):

    pub fn cookie(&mut self, cookie: cookie::Cookie<'_>) -> &mut Self {
        match cookie.to_string().try_into_value() {
            Ok(hdr_val) => self.append_header((header::SET_COOKIE, hdr_val)),
            Err(err) => {
                self.error = Some(err.into());
                self
            }
        }
    }

My thought:

    pub fn cookie(&mut self, cookie: cookie::Cookie<'_>) -> &mut Self {
        self.append_header((header::SET_COOKIE, cookie.to_string()))
    }

Is there any reason to do this conversion?

[Thomblin]

To which cookie function do you refer? I implemented this according to https://docs.rs/actix-web/latest/actix_web/struct.HttpResponse.html#method.add_cookie which performs the same check to validate the cookie

[kukuro-kt]

To which cookie function do you refer?

This one: https://docs.rs/actix-web/latest/actix_web/struct.HttpResponseBuilder.html#method.cookie

By the way, this implementation saves error to self instead of returning a Result

[Thomblin]

self.append_header expects hdr_val to implement TryIntoHeaderPair which returns (HeaderName, HeaderValue), so in the end it is the same as HeaderValue::from_str if I understand that correctly

self.append_header((header::SET_COOKIE, cookie.to_string())) should not work either as impl<V> TryIntoHeaderPair for (String, V) returns Result<(HeaderName, HeaderValue), Self::Error>

[kukuro-kt]

I mean, although customize() has not supported cookie() or add_cookie() yet, there is still a simple way to add cookie:

actix_web::HttpResponse::Ok()
  .json(...)
  .customize()
  .append_header((header::COOKIE, cookie.to_string())); //no need to unwrap() or `?`

append_header() provided by CustomizeResponder does not return a Result but saves error to self:

Err(err) => self.error = Some(err.into()),

However, the add_cookie() you implemented returns a Result, so we have to write like this:

actix_web::HttpResponse::Ok()
  .json(...)
  .customize()
  .add_cookie(cookie)?; //need to unwrap() or `?`

Return a Result here may not be in line with the api design concept of CustomizeResponder?

[Thomblin]

I see your point. I guess it depends if you prefer to fail silently or not. As far as I understand Rust, the idea is usually to make possible errors transparent by using the Result type. I don't know why there are two different implementations to add a cookie