Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse cookies into cookiejar. set cookies from cookiejar #64

Closed
yoshuawuyts opened this issue Jan 21, 2020 · 1 comment
Closed

parse cookies into cookiejar. set cookies from cookiejar #64

yoshuawuyts opened this issue Jan 21, 2020 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Jan 21, 2020

After having reviewed http-rs/tide#380, I'm now wondering if we should have a different approach for cookies than what we currently have. This supersedes #48.

Current API

We currently provide APIs for both getting and setting cookies in the request and response headers. Each call to e.g. Response::cookie decodes the full set of cookies, and returns the corresponding cookie:

https://github.com/yoshuawuyts/http-types/blob/b1623854102986b151d07406b6949d985787d1fd/src/response.rs#L305-L314

https://github.com/yoshuawuyts/http-types/blob/b1623854102986b151d07406b6949d985787d1fd/src/response.rs#L331-L335

This has the downside of being rather slow, as each access of a cookie ends up going through a full decode stage.

Proposed API

In http-rs/tide#380 Request has cookie,set_cookie, and remove_cookie methods that only parse cookies once. And even better: it automatically sets the correct Set-Cookie headers on the Response by calculating the delta of the cookies that have been set before, and which need to be set again.

To make that API easier to write, I'd like to propose we change the current cookies API in http-types to primarily operate on CookieJar instead.

impl Request {
    /// Parse the cookies from the headers into a `CookieJar`
    pub fn cookies(&self) -> CookieJar;

    /// Encodes the delta of the cookies in the `CookieJar` as the corresponding HTTP headers.
    pub fn set_cookies(&mut self, jar: &CookieJar);
}

impl Response {
    /// Parse the cookies from the headers into a `CookieJar`
    pub fn cookies(&self) -> CookieJar;

    /// Encodes the delta of the cookies in the `CookieJar` as the corresponding HTTP headers.
    pub fn set_cookies(&mut self, jar: &CookieJar);
}

This way http-types can operate on cookie jars, rather than on individual cookies. And as we can see in Tide it would then be possible to at the framework layer enable operating on individual cookies instead.


cc/ @tirr-c @rylev

@yoshuawuyts yoshuawuyts added the enhancement New feature or request label Mar 3, 2020
@yoshuawuyts yoshuawuyts added this to the breaking milestone Apr 16, 2020
@yoshuawuyts
Copy link
Member Author

Not sure if this is right. We should experiment more in Tide / Surf before we revisit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant