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

support for chunked cookies #35

Closed
zandbelt opened this issue Feb 10, 2017 · 7 comments
Closed

support for chunked cookies #35

zandbelt opened this issue Feb 10, 2017 · 7 comments

Comments

@zandbelt
Copy link
Contributor

Using the default cookie storage it may happen that the size of the cookie runs over browser limits when a lot of data is stored in the session. Adding support for chunked cookies would mitigate this somewhat. See: zmartzone/lua-resty-openidc#33

@bungle
Copy link
Owner

bungle commented Feb 10, 2017

@zandbelt, Thanks. I saw that report. We should consider adding this support to lua-resty-session. Do you happen to know if there is any "standard" of how this should be handled?

@zandbelt
Copy link
Contributor Author

I don't think there's a real standard; it doesn't really need one because its all "internal"; the process would be:

  1. determine that the cookie size is too large to put in a single cookie (e.g. size > MAX bytes)
  2. determine the number of cookies that should be used (e.g. n = size / MAX)
  3. write a cookie that contains the number of chunks (e.g. Set-Cookie: session_number=)
  4. write the cookies that contain the content (e.g. Set-Cookie: session_1=..., Set-Cookie: session_2=..., Set-Cookie: session_=...)

On retrieval one would check whether a cookie named session_number exists and would try read that many cookies whilst concatenating the content, otherwise fallback to the current behavior.

For an idea of the effort, see:
OpenIDC/mod_auth_openidc@5b6bb06

There's still a limitation in the maximum size of the (single) "Cookie" header that the browser will actually manage to send, but that is known to be much larger (typically at least 81920), see:
http://browsercookielimits.squawky.net/

@bungle
Copy link
Owner

bungle commented Feb 10, 2017

Okay. I can think about implementing this. Thank you. I already started looking at it.

@bungle
Copy link
Owner

bungle commented Feb 10, 2017

@zandbelt,

The first try to do this is implemented in these commits:
b641e4b
99407c9
3494b16

BEWARE!

I just wrote the code, but I have not run it. That means that there could be some nasty bugs. Also in this version the data part of the cookie is limited to fixed 4000 chars in each chunk (we can add configuration later), to make room for name and flags.

I don't have time to test this thoroughly right now. But all the feedback is welcomed.

@bungle
Copy link
Owner

bungle commented Feb 10, 2017

I made some obvious fixes after the comment above.

@bungle
Copy link
Owner

bungle commented Feb 11, 2017

@zandbelt,

Okay, it got some nasty bugs, but those are resolved now.

Can you please try it out and report back to me if it works as expected.

Note: please also adjust your Nginx config as needed, e.g. to accept larger cookies, you may need to add something like this in nginx.conf:

large_client_header_buffers 4 16k;

@bungle
Copy link
Owner

bungle commented Feb 13, 2017

This is now released as a part of 2.15 release.

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

No branches or pull requests

2 participants