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

session: unnecessary `Set-Cookie` in certain web pages #2291

Closed
tiborsimko opened this Issue Sep 17, 2014 · 11 comments

Comments

Projects
None yet
3 participants
@tiborsimko
Member

tiborsimko commented Sep 17, 2014

Some web pages in next use Set-Cookie unnecessarily, for example the "page not found":

$  curl -I http://0.0.0.0:8080/thisdoesnotreallyexist
HTTP/1.0 404 NOT FOUND
Content-Type: text/html; charset=utf-8
Content-Length: 9669
Set-Cookie: session=81ff5395829c4f1488d661d96bde137d; HttpOnly; Path=/
Server: Werkzeug/0.9.6 Python/2.7.6
Date: Wed, 17 Sep 2014 12:37:41 GMT

It would be good to set cookies only on pages where this is really required.

BTW In master things are not perfect either; while /thisdoesnotreallyexist is OK, /help/* is not, even though there is nothing user-specific there for guests.

@tiborsimko

This comment has been minimized.

Member

tiborsimko commented Sep 19, 2014

It would be good to generalise this so that when a user is guest, then no cookie is set on any page.
(modulo INSPIRE paper claiming that still needs guest sessions... CC-ing @kaplun @glouppe)

@jirikuncar jirikuncar added this to the v2.0.x milestone Sep 19, 2014

@jirikuncar

This comment has been minimized.

Member

jirikuncar commented Sep 19, 2014

We should check if we can create Guest user without session id.

@kaplun

This comment has been minimized.

Member

kaplun commented Sep 22, 2014

... when you say "create Guest user" I guess this does not involve allocating a user on the DB, correct?
But is it really worth not always having cookies? Anyway sessions are now cheap thanks to redis...

@tiborsimko

This comment has been minimized.

Member

tiborsimko commented Sep 22, 2014

But is it really worth not always having cookies? Anyway sessions are now cheap thanks to redis...

"Cheap" is relative, e.g. during slashdot effect times it would be expensive to create new sessions for guest users as they are hitting our site. We would be bringing unto ourselves a completely unnecessary scalability problem in this way -- unnecessary because we are serving any guest user the very same content (modulo paper claiming). Varnish could take care of guest slashdotters nicely; however it cannot if we differentiate between guest user and a guest user.

(Post scriptum: in master we used to have CFG_WEBSESSION_DIFFERENTIATE_BETWEEN_GUESTS that is by default set to zero -- but actually due to paper claiming changes, we are not respecting this setting anymore... I would consider this a regression. We could open another ticket for this.)

@kaplun

This comment has been minimized.

Member

kaplun commented Sep 23, 2014

(Post scriptum: in master we used to have CFG_WEBSESSION_DIFFERENTIATE_BETWEEN_GUESTS that is by default set to zero -- but actually due to paper claiming changes, we are not respecting this setting anymore... I would consider this a regression. We could open another ticket for this.)

Nope there is no regression: that flag was meant to allocate guest users in the user table Vs. serving always userid 0. This is still respected. What is happens currently in bibauthorid (but in fact potentially anywhere) is that a session (and the corresponding cookie) is preserved server-side, if and only if the session is storing some valuable data where valuable is defined in:
https://github.com/inveniosoftware/invenio/blob/master/modules/websession/lib/session.py#L276

So what happens today is that a cookie is always created, but the corresponding session is discarded in case there is nothing useful to store.

@tiborsimko

This comment has been minimized.

Member

tiborsimko commented Sep 23, 2014

So what happens today is that a cookie is always created...

... which should not be necessary in my eyes. If we eliminate these, we'll be much more scalable. Currently we produce lots of these for every single (even homepage) hit:

$ for in in 0 1 2 3; do curl -I http://inspirehep.net/; done
HTTP/1.1 200 OK
Date: Tue, 23 Sep 2014 08:33:47 GMT
Server: Apache
Cache-Control: public, max-age=3600, no-cache="set-cookie"
Vary: Cookie,ETag,Cache-Control,User-Agent
Content-Type: text/html; charset=UTF-8
Set-Cookie: INVENIOSESSIONstub=NO; path=/
Set-Cookie: INVENIOSESSION=b4abd3dd761b5a1bd8ae3a34a8d30f17; path=/

HTTP/1.1 200 OK
Date: Tue, 23 Sep 2014 08:33:47 GMT
Server: Apache
Cache-Control: public, max-age=3600, no-cache="set-cookie"
Vary: Cookie,ETag,Cache-Control,User-Agent
Content-Type: text/html; charset=UTF-8
Set-Cookie: INVENIOSESSIONstub=NO; path=/
Set-Cookie: INVENIOSESSION=2c256f7b75568a69f5b486eb9d865367; path=/

HTTP/1.1 200 OK
Date: Tue, 23 Sep 2014 08:33:47 GMT
Server: Apache
Cache-Control: public, max-age=3600, no-cache="set-cookie"
Vary: Cookie,ETag,Cache-Control,User-Agent
Content-Type: text/html; charset=UTF-8
Set-Cookie: INVENIOSESSIONstub=NO; path=/
Set-Cookie: INVENIOSESSION=700b731a9ac32aae70592c92d99642d0; path=/

HTTP/1.1 200 OK
Date: Tue, 23 Sep 2014 08:33:47 GMT
Server: Apache
Cache-Control: public, max-age=3600, no-cache="set-cookie"
Vary: Cookie,ETag,Cache-Control,User-Agent
Content-Type: text/html; charset=UTF-8
Set-Cookie: INVENIOSESSIONstub=NO; path=/
Set-Cookie: INVENIOSESSION=bffaad1f86663a9c8652d2199071bd8a; path=/
@kaplun

This comment has been minimized.

Member

kaplun commented Sep 23, 2014

Yes, but how would you do if all of a sudden the guest user is doing something that requires statefullness? Or are we really implying guests users should not me allowed doing any statefull operation?

@jirikuncar

This comment has been minimized.

Member

jirikuncar commented Sep 23, 2014

Or are we really implying guests users should not me allowed doing any stateful operation?

IMHO guest user doesn't need any stateful operation. If you need to save some state, ask him to log in before you save the state. Do you have any specific use-case that can't be solved otherwise?

@tiborsimko

This comment has been minimized.

Member

tiborsimko commented Sep 23, 2014

Or are we really implying guests users should not me allowed doing any statefull operation?

Besides paper claiming, what are the needs? E.g. next/previous hit on detailed record search pages comes to mind, but this is something we can live without. In any case, we'll have this configured by a variable, so each site could decide to go one way or another...

@kaplun

This comment has been minimized.

Member

kaplun commented Sep 23, 2014

IMHO guest user doesn't need any stateful operation. If you need to save some state, ask him to log in before you save the state. Do you have any specific use-case that can't be solved otherwise?

Well, unless you have a web app and everything is stored client side, as soon as you ask the user to log in you loose the state, unless it's all done via AJAX. But we are not yet at that stage.

Besides paper claiming, what are the needs? E.g. next/previous hit on detailed record search pages comes to mind, but this is something we can live without. In any case, we'll have this configured by a variable, so each site could decide to go one way or another...

Indeed these are the only two cases so far.

In principle, anyway, when the server has sent all the headers, it should already have decided whether there is anything sensible to store in a session or not (at least in Flask), so indeed at that point it can avoid issuing a cookie in case the session is not valuable.

jirikuncar added a commit to jirikuncar/invenio that referenced this issue Oct 22, 2014

session: unnecessary Set-Cookie removal
* Removes unnecessary writes to session for guest user.  (closes inveniosoftware#2291)

Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>

jirikuncar added a commit to jirikuncar/invenio that referenced this issue Oct 22, 2014

session: unnecessary Set-Cookie removal
* Removes unnecessary writes to session for guest user.  (closes inveniosoftware#2291)

Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>

jirikuncar added a commit to jirikuncar/invenio that referenced this issue Oct 22, 2014

session: removal of unnecessary Set-Cookie
* Removes unnecessary writes to session for guest user.  (closes inveniosoftware#2291)

Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>

jirikuncar added a commit to jirikuncar/invenio that referenced this issue Oct 28, 2014

session: removal of unnecessary Set-Cookie
* Removes unnecessary writes to session for guest user.  (closes inveniosoftware#2291)

Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>

jirikuncar added a commit to jirikuncar/invenio that referenced this issue Oct 28, 2014

session: removal of unnecessary Set-Cookie
* Removes unnecessary writes to session for guest user.  (closes inveniosoftware#2291)

Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>
@jirikuncar

This comment has been minimized.

Member

jirikuncar commented Oct 28, 2014

Closed via #2445.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment