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

Use an unpadded session. #2391

Merged
merged 1 commit into from
Apr 21, 2024
Merged

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Apr 6, 2024

This changes the serialization method used by Mojo::Sessions to merely JSON encode the session without padding. The default serialization method JSON encodes the session and then pads it with the letter Z to a length of 1025 characaters.

This results in much smaller session cookies (roughly one fifth the previous size). Thus it is possible to be signed in to more courses at a time without hitting the cookie size limit.

This should perhaps only be considered if the limit on the number of courses that can be signed into at one time is deemed to be a big enough problem that something needs to be done.

@Alex-Jordan
Copy link
Contributor

I'm hitting this issue a lot, but of course it is because I've been logging into other people's courses to help with this and that.

There's one instructor here with two sections of a course, and this instructor likes to use one WW course for homework and a separate one for quizzes. They have certain global config settings different and find it easier to shut off access to HW that way during a scheduled test. So this instructor has four WW courses, which is the limit under this issue. It wouldn't be hard to imagine a need for this instructor to have a fifth one.

So I think this is a good change. I'll try it out soon.

Copy link
Contributor

@Alex-Jordan Alex-Jordan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this, I am able to log in to at least 12 courses without any issues. Not saying that is the cap I experienced, just that's how many I tried. Without this, it was usually 4. Sometimes it felt like I could enter a 5th, but then not succeed at navigating within that 5th.

@drgrice1 drgrice1 force-pushed the session-not-padded branch 3 times, most recently from c11897c to 2ae271a Compare April 14, 2024 10:36
This changes the serialization method used by `Mojo::Sessions` to merely
JSON encode the session without padding.  The default serialization
method JSON encodes the session and then pads it with the letter `Z`
to a length of 1025 characaters.

This results in much smaller session cookies (roughly one fifth the
previous size).  Thus it is possible to be signed in to more courses at
a time without hitting the cookie size limit.

This should perhaps only be considered if the limit on the number of
courses that can be signed into at one time is deemed to be a big enough
problem that something needs to be done.
@drgrice1 drgrice1 force-pushed the session-not-padded branch from 2ae271a to 37f3262 Compare April 17, 2024 15:18
@drgrice1
Copy link
Member Author

I think that we should go with this pull request. To be honest, I don't understand what the advantage of padding the session is, and I don't see any real downside to removing the padding. It seems that the padding just artificially inflates the cookie size.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test with many courses, but I think let's merge.

@pstaabp pstaabp merged commit 608b68a into openwebwork:develop Apr 21, 2024
2 checks passed
@drgrice1 drgrice1 deleted the session-not-padded branch April 21, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants