-
Notifications
You must be signed in to change notification settings - Fork 5
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
Document deprecation of this package. #14
Conversation
I would like to recommend alternatives in the readme, but I don't really know what a valid recommendation would be. Suggestions are welcome. :-) |
?rekcäH nitraM? wrote at 2019-8-11 23:28 -0700:
I would like to recommend alternatives in the readme, but I don't really know what a valid recommendation would be. Suggestions are welcome. :-)
Instead of a session use cookies -- if possible.
Most server side session implementations are vulnerable to
DOS attacks (any session needs resources, and it is often
fairly easy for an attacker to make the server create new sessions).
The use of cookies prevents this kind of attack (as cookies
are stored on the client).
If you need a session, put the session data into a normal storage
(rather than a `temporarystorage`).
I will soon publish `dm.zope.session` (as an alternative to
`Products.Transience`). It will be less prone to `ConflictError` if
intensively used (e.g. by every request) and supports features
for (custom) detection (and maybe handling) of session based DOS attacks.
It also sketches the necessary steps for its use (many of them
similar to those necessary to use the standard Zope sessions
on a "normal" storage.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with deprecating the package.
Please also add a note to the change log.
Could you please incorporate the suggestions of @d-maurer for alternatives?
Apply suggestion Co-Authored-By: Michael Howitz <mh@gocept.com>
Do these changes address the raised concerns? I have also updated #15 to include these changes. I really think there should be a warning / log entry too so people still using this package have a chance to discover that it actually got deprecated (and I'm probably doing it wrong to warn them like I do, so some feedback there is also really appreciated). |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
According to #8 and #12 much of the discussion that is linked in these issues this package is broken and not usable as is (and probably hard / impossible to fix without a big investment).
Since this is known by many long time community members, this should be documented at least in the readme, so that other community members have a chance to find out about this fact more easily.