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

Fixed #24468 -- Made signed cookies cache backend resilient to unpickling exceptions. #4299

Merged
merged 1 commit into from Mar 12, 2015

Conversation

Projects
None yet
2 participants
@timgraham
Member

timgraham commented Mar 11, 2015

No description provided.

except (signing.BadSignature, ValueError):
except Exception:
# BadSignature, ValueError, or unpickling exceptions. If any of
# these happen, reset the session.

This comment has been minimized.

@charettes

charettes Mar 11, 2015

Member

I couldn't find a valid way of making the Exception catching cloak possible misconfiguration but I wonder if it wouldn't be better to make session serializers re-raise a common DeserializationError and catch it here.

@charettes

charettes Mar 11, 2015

Member

I couldn't find a valid way of making the Exception catching cloak possible misconfiguration but I wonder if it wouldn't be better to make session serializers re-raise a common DeserializationError and catch it here.

This comment has been minimized.

@timgraham

timgraham Mar 11, 2015

Member

My justification is that we are catching Exception in similar places of other cache backends. If it ends up hiding actual problems, maybe a solution could be to add debug logging.

@timgraham

timgraham Mar 11, 2015

Member

My justification is that we are catching Exception in similar places of other cache backends. If it ends up hiding actual problems, maybe a solution could be to add debug logging.

This comment has been minimized.

@charettes

charettes Mar 11, 2015

Member

I think debug logging would be fine here.

@charettes

charettes Mar 11, 2015

Member

I think debug logging would be fine here.

This comment has been minimized.

@timgraham

timgraham Mar 11, 2015

Member

Are you okay to defer it until someone reports an actual problem? Also, I question the usefulness of adding more logging until we change Django's default logging config so logged statements are actually displayed somewhere.

@timgraham

timgraham Mar 11, 2015

Member

Are you okay to defer it until someone reports an actual problem? Also, I question the usefulness of adding more logging until we change Django's default logging config so logged statements are actually displayed somewhere.

This comment has been minimized.

@charettes

charettes Mar 12, 2015

Member

Ship it!

@charettes

charettes Mar 12, 2015

Member

Ship it!

@timgraham timgraham merged commit 8a48149 into django:master Mar 12, 2015

3 of 4 checks passed

default Merged build started.
Details
docs Merged build finished.
Details
flake8 Merged build finished.
Details
isort Merged build finished.
Details

@timgraham timgraham deleted the timgraham:24468 branch Mar 14, 2015

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