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

get_cookie_name in SessionInterface for easier overriding in SecureCookieSessionInterface #3369

Merged
merged 1 commit into from
Oct 13, 2019

Conversation

doronhorwitz
Copy link
Contributor

@doronhorwitz doronhorwitz commented Sep 22, 2019

Most of the other arguments passed into response.set_cookie() in SecureCookieSessionInterface.save_session() use the overridable methods of SessionInterface to fill in each argument. Except for the cookie name, which is statically set to app.session_cookie_name.

Sometimes a cookie name needs to be set dynamically based on the current request, for example, a cookie name that is dependent on the current subpath in the URL. At the moment, the best way of doing this is to override the save_session() method of SecureCookieSessionInterface and make it dynamically pass in the cookie name to response.set_cookie().

To avoid having to override the whole of save_session(), this pull request adds in an overridable get_cookie_name() method on SessionInterface to match the other get_xxx() methods of that class.

Note: I originally posted a question about this in Stackoverflow ("Something like get_cookie_name in Flask's SessionInterface?") and also asked the question (using the Stackoverflow link) in the Flask Discord chat. I was not satisfied with the response I got, so I've opened up this pull request.

@davidism
Copy link
Member

Makes sense. Needs a changelog and tests.

@doronhorwitz doronhorwitz force-pushed the get-cookie-name branch 5 times, most recently from 6f328f4 to 9d640ae Compare September 24, 2019 11:39
@doronhorwitz
Copy link
Contributor Author

Thanks @davidism. Added a test and changelog.

@doronhorwitz
Copy link
Contributor Author

doronhorwitz commented Oct 2, 2019

@davidism, is my test and changelog adequate? Is there anything more required?

@davidism
Copy link
Member

davidism commented Oct 2, 2019

Looks good, I'll get to it when I can.

@doronhorwitz
Copy link
Contributor Author

great, thanks.

@davidism davidism merged commit 200ce9b into pallets:master Oct 13, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
@doronhorwitz doronhorwitz deleted the get-cookie-name branch January 26, 2021 12:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants