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

Introduce session objects #2115

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vondele
Copy link
Member

@vondele vondele commented Jul 9, 2024

The fishtest remote is handled with a session object, which allows one to persist parameters between requests, and in particular to maintain tcp connections.

@vondele vondele marked this pull request as draft July 9, 2024 07:12
The fishtest remote is handled with a session object,
which allows one to persist parameters between requests,
and in particular to maintain tcp connections.
@peregrineshahin
Copy link
Contributor

peregrineshahin commented Jul 19, 2024

I think this is too related to the password hashing problem, you can login once and keep reusing the session.
With it we can probably save the signed-cookies from the server and reuse it instead of the revalidation and rehashing of passwords with each request with no cache, but there is a big concern for that; browsers forbid the client to control such requests, but on a python program/curling a misuser can change the expiration cookie time.
Thus they can bybass blocking, the good news though is that instead of a signed cookie we can give the worker a token and maintain a storage of the valid sessions so the server can invalidate a session not via a new "forgetten" subsequent response, but by invalidating the session on the server.

@vondele
Copy link
Member Author

vondele commented Jul 20, 2024

The primary purpose of the PR is to improve performance, i.e. ensure connections to the server are persistent. This is also working, but we would need to understand if there is a disadvantage to configuring the server such that it has 10k+ persistent open connections (we know how, but not if it is a good idea.) That's actually a question @noobpwnftw might be able to answer.

@vondele
Copy link
Member Author

vondele commented Jul 20, 2024

Apart from that, I agree that it could be used to store tokens and change authentication. But it is a separate topic that would need further work.

@peregrineshahin
Copy link
Contributor

peregrineshahin commented Jul 20, 2024

Probably can also close the session in case graceful shutdown with session.close() or similar

@noobpwnftw
Copy link
Contributor

It is generally not a good idea to have too many persistent connections especially when there is a proxy on the same box. OTOH, the client side session objects can be safely added without worrying about underlying work, web servers today are smart enough to deal with them(shorten or turn off keepalives from server configs when in doubt).

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

Successfully merging this pull request may close these issues.

3 participants