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

Bug: ASGIConnection.session incorrect type - may return Empty #2435

Open
peterschutt opened this issue Oct 13, 2023 · 1 comment
Open

Bug: ASGIConnection.session incorrect type - may return Empty #2435

peterschutt opened this issue Oct 13, 2023 · 1 comment

Comments

@peterschutt
Copy link
Contributor

peterschutt commented Oct 13, 2023

If setting scope["session"] to Empty is the same as the session not being set, then perhaps we should change ASGIConnection.session to raise if it is accessed when scope["session"] == Empty. At least that way we are sure to honor the type of the property. E.g.,:

    @property
    def session(self) -> dict[str, Any]:
        """Return the session for this connection if a session was previously set in the ``Scope``

        Returns:
            A dictionary representing the session value - if existing.

        Raises:
            ImproperlyConfiguredException: if session is not set in scope.
        """
        if "session" not in self.scope or self.scope["session"] is Empty:  # <---- add this "or" clause
            raise ImproperlyConfiguredException(
                "'session' is not defined in scope, install a SessionMiddleware to set it"
            )

        return cast("dict[str, Any]", self.scope["session"])

Originally posted by @peterschutt in #2427 (comment)

Note

Check out all issues funded or available for funding here: https://polar.sh/litestar-org

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@peterschutt
Copy link
Contributor Author

The potential Empty value for connection.session comes from:

if not connection.session or connection.scope["session"] is Empty:
# the assignment of 'Empty' forces the session middleware to clear session data.
connection.scope["session"] = Empty
raise NotAuthorizedException("no session data found")

peterschutt added a commit that referenced this issue Oct 14, 2023
Property is typed to return `dict[str, Any]` but we use `Empty` as a sentinel to instruct session middleware to clear the session.

This PR changes the behavior of `ASGIConnection.session` to raise an exception on session access, just as if the session key is not set at all.

In addition, includes some related refactorings:

- Defines a new constant, `SCOPE_SESSION_KEY: Final = "session"` and replaces uses of `"session"` literal with that.
- Refactors tests/unit/test_connection/test_base.py to utilise fixtures.

Closes #2435
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging a pull request may close this issue.

1 participant