Closed thumbnail closed 1 year ago
Thanks for raising this issue. My thought for resolving it would be to add an option :set-cookies?
to the session middlewware that defaults to true
, but can be set to false
in order to disable this behaviour.
Of course, the ideal would be to not have the session and cookie middleware complected to begin with, but unfortunately we have to live with past mistakes to ensure backward compatibility. However, we may be able to solve this issue in Ring 2.0.
My thought for resolving it would be to add an option :set-cookies? to the session middleware that defaults to true, but can be set to false in order to disable this behaviour.
I'd be happy to contribute such an option if want! just LMK.
but unfortunately we have to live with past mistakes to ensure backward compatibility.
Yeah this was my concern too. Another possible solution would to create a new namespace. (lacinia-pedestal did a similar thing a while back).
However, we may be able to solve this issue in Ring 2.0.
In the issue about ring 2.0 you mention it should maintain 1.0 compatibility, wouldn't that be the same issue?
On a separate thought; Originally I wanted to encrypt the cookies (very similar to the session.cookie store). I ended up implementing a very similar solution as cookie encoder/decoder, but it would be nice to extract the encryption from ring.middleware.session.cookie
into separate encoders/decoders instead. WDYT?
I'd be happy to contribute such an option if want! just LMK.
Sure. PRs are welcome.
In the issue about ring 2.0 you mention it should maintain 1.0 compatibility, wouldn't that be the same issue?
For performance purposes, I was considering ring.middleware2.*
namespaces that contain middleware that will only work with the updated request maps, and as the namespace differs, we can fix any legacy issues with these functions as well. The ring.middleware
namespace is intended work with both request maps, and should be backward compatible.
On a separate thought; Originally I wanted to encrypt the cookies (very similar to the session.cookie store). I ended up implementing a very similar solution as cookie encoder/decoder, but it would be nice to extract the encryption from
ring.middleware.session.cookie
into separate encoders/decoders instead. WDYT?
My first thought is that it might be too niche a requirement to have as part of Ring core.
Sure. PRs are welcome.
Allright, I have a fix locally will create the PR when I have a test ready too!
My first thought is that it might be too niche a requirement to have as part of Ring core.
That seems fair 👍 . it's more about decomplecting the session.cookie store. Moving the encryption to cookie-encoders/decoder lowers the responsibility for the session-stores too. By splitting the two, ring allows consumers to use different session-stores (without duplicating the encryption logic), or change the encryption logic without duplicating the session store.
But I agree that it's a niche ;). I just noticed I needed to duplicate the encryption while building a database-backed session-store.
Note: A fix for this issue should also resolve #392.
Initially asked on slack
When combining
middleware.session
andmiddleware.cookie
with custom options, the encoding/decoding doesn't work as expected. Either no cookies get encoded, or none get decoded. It seems to happen because the session-middleware manually invokes cookie-middleware functions instead of relying on the chain.We need this in our app to add signatures to all cookies; without making it a concern of each handler.
Repro
Workaround
If the cookie-middleware is removed from the session-middleware by removing these lines: https://github.com/ring-clojure/ring/blob/ac821a51d0d30f97f3471f026bb4898bebaaae94/ring-core/src/ring/middleware/session.clj#L75 https://github.com/ring-clojure/ring/blob/ac821a51d0d30f97f3471f026bb4898bebaaae94/ring-core/src/ring/middleware/session.clj#L40
the session-middleware and cookie-middleware play along nicely