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.

Sign up for GitHub

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

Jump to bottom

seems response header always has set-cookie, even use same session #85

Open
yurenchen000 opened this issue Feb 1, 2022 · 6 comments · May be fixed by #90
Open

seems response header always has set-cookie, even use same session #85

yurenchen000 opened this issue Feb 1, 2022 · 6 comments · May be fixed by #90

Comments

@yurenchen000
Copy link

yurenchen000 commented Feb 1, 2022 •
edited

use the aioredis demo:
https://sanic-session.readthedocs.io/en/latest/using_the_interfaces.html#redis-aioredis

and response header always has set-cookie, even use same session
// event comment out foo +=1

Is this normal?
// use set-cookie only when cookie has change ?

detail

version

  • sanic 21.6.2
  • sanic-session 0.8.0
  • python 3.9

chrome test

sanic_session_cookie__.png



curl test:

$ curl -sv 'http://localhost:8006/' --cookie 'session=ac97cd2a67e54328a6d06ba8e471cacd'

> GET / HTTP/1.1
> Host: localhost:8006
> User-Agent: curl/7.68.0
> Accept: */*
> Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd
> 

< HTTP/1.1 200 OK
< Set-Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd; Path=/; HttpOnly; expires=Thu, 03-Mar-2022 20:23:27 GMT; Max-Age=2592000
< content-length: 38
< connection: keep-alive
< content-type: text/plain; charset=utf-8
< 
foo: <SessionDict {'foo': {'val': 0}}>


$ curl -sv 'http://localhost:8006/' --cookie 'session=ac97cd2a67e54328a6d06ba8e471cacd'

> GET / HTTP/1.1
> Host: localhost:8006
> User-Agent: curl/7.68.0
> Accept: */*
> Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd
> 

< HTTP/1.1 200 OK
< Set-Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd; Path=/; HttpOnly; expires=Thu, 03-Mar-2022 20:23:29 GMT; Max-Age=2592000
< content-length: 38
< connection: keep-alive
< content-type: text/plain; charset=utf-8
< 
foo: <SessionDict {'foo': {'val': 0}}>


$ curl -sv 'http://localhost:8006/' --cookie 'session=ac97cd2a67e54328a6d06ba8e471cacd'

> GET / HTTP/1.1
> Host: localhost:8006
> User-Agent: curl/7.68.0
> Accept: */*
> Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd
> 

< HTTP/1.1 200 OK
< Set-Cookie: session=ac97cd2a67e54328a6d06ba8e471cacd; Path=/; HttpOnly; expires=Thu, 03-Mar-2022 20:23:30 GMT; Max-Age=2592000
< content-length: 38
< connection: keep-alive
< content-type: text/plain; charset=utf-8
< 
foo: <SessionDict {'foo': {'val': 0}}>

every request will got cookie with new expires value.
@yurenchen000
Copy link
Author

yurenchen000 commented Feb 1, 2022 •
edited

possible workaround

diff sanic_session/base.py

    def _set_cookie_props(self, request, response):
        req = get_request_container(request)
+        if request.cookies.get(self.cookie_name) == req[self.session_name].sid:
+            return   # session_id same with client, do nothing
+
        response.cookies[self.cookie_name] = req[self.session_name].sid
        response.cookies[self.cookie_name]["httponly"] = self.httponly

@ahopkins
Copy link
Owner

Is this still an issue?

@yurenchen000
Copy link
Author

yurenchen000 commented Dec 28, 2022 •
edited

Is this still an issue?

that not really break normal function
It's just that the browser cookies always refreshed (after each request)

maybe it's by design?

I see other webserver not refresh cookie so frequently
(but if not refresh after each request, It still needs to be refreshed at certain time, in order to renew the cookie time, maybe)

@ahopkins
Copy link
Owner

ahopkins commented Dec 28, 2022 •
edited

Ahh, I see. I am new to this lib, so I cannot speak to the design decisions. Maybe that should be a configurable setting.

@yurenchen000
Copy link
Author

yurenchen000 commented Dec 28, 2022 •
edited

Ahh, I see. I am new to this lib, so I cannot speak to the design decisions. Maybe that should be a configurable setting.

about cookie login expire time, I see there are some different design:

  • some server never renew cookie expire time, expire based on login time
  • some server renew cookie at some time points, maybe like DHCP lease renewal.
    //seems few servers refresh cookies at each request

@ahopkins
Copy link
Owner

That makes sense. I guess what I am caught up on is that I have not thought thru the consequences of this and how changing the behavior would impact existing installations.

@ahopkins ahopkins linked a pull request Dec 29, 2022 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Default to no cookie refresh ahopkins/sanic-session
2 participants
@ahopkins @yurenchen000