Marking flushed sessions as not modified

[Andrew Godwin]

An interesting bug has turned up in Channels where someone is trying to remove a session using .flush(), but our decorator sees that it's been modified and then re-saves it, thinking it needs to persist data.

What are people's opinions on changing flush() to also set modified=False on sessions? Is there a bad side-effect of this I haven't considered?

Andrew

[Tim Graham]

I'm not sure offhand. Could you provide a link to the issue so we could look at the details?

[Andrew Godwin]

Here's the Channels issue underlying it: https://github.com/django/channels/issues/477

For quick reference, the @channel_session decorator is something that wraps a consumer, loading a session on the way in and saving it on the way out if it changed, similar to how session middleware might work.

Andrew

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/2ae6fbc5-171f-4647-9604-29fc6833ee0a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Tim Graham]

The change you proposed might be acceptable, although I'm a bit nervous about breaking backwards-compatibility in some unforeseen use case. I wonder if modifying the condition in the
channel_session decorator to: if session.modified and not session.is_empty(): would solve the issue? This seems similar to what the SessionMiddleware does.

On Monday, January 23, 2017 at 2:59:59 PM UTC-5, Andrew Godwin wrote:

Here's the Channels issue underlying it: https://github.com/django/channels/issues/477

For quick reference, the @channel_session decorator is something that wraps a consumer, loading a session on the way in and saving it on the way out if it changed, similar to how session middleware might work.

Andrew

On Mon, Jan 23, 2017 at 11:57 AM, Tim Graham <timog...@gmail.com> wrote:

I'm not sure offhand. Could you provide a link to the issue so we could look at the details?

On Monday, January 23, 2017 at 1:45:48 PM UTC-5, Andrew Godwin wrote:

An interesting bug has turned up in Channels where someone is trying to remove a session using .flush(), but our decorator sees that it's been modified and then re-saves it, thinking it needs to persist data.

What are people's opinions on changing flush() to also set modified=False on sessions? Is there a bad side-effect of this I haven't considered?

Andrew

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.

To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.

[Andrew Godwin]

Yes, that's probably the best short-term fix and will keep it working with older versions - I'll put that in as a fix for this now.

I agree it's potentially dangerous, but then I also think the current behaviour is a bit misleading. I'll dig around some third party apps and see if I can see examples of how people call it.

Andrew

To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.

Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/41a2b00e-a40a-4b45-9936-741cc58d2aed%40googlegroups.com.