Improvements to Session State code #536
Conversation
|
|
||
| // EncryptInto encrypts the value and stores it back in the string pointer | ||
| func (c *Cipher) EncryptInto(s *string) error { | ||
| return into(c.Encrypt, s) |
There was a problem hiding this comment.
@JoelSpeed In my size reduction analysis in #523 -- this technique was the biggest culprit of the bloat. Encrypt having an embedded base64 encoding so the result was a valid string that could be in JSON causes 33% size growth.
This is why I experimented shifting to MessagePack (since it supports internal binary data) and relatedly, moving the encryption after the fact to the whole payload, rather than encrypting inside the fields (for speed and to save a few bytes of AES-CFB IV headers present for each distinct inner encryption, especially for smaller non token fields).
There was a problem hiding this comment.
Ack. This PR isn't aimed at improving the size of the sessions at all but is more aimed at improving the code quality. I imagine what will happen is a lot of this may be undone once we implement the MessagePack route, but for now, it shows progression towards better code and eventually the better session sizes
JoelSpeed
force-pushed
the
session-state-improvements
branch
from
05abdcc to
7f69266
Compare
JoelSpeed
force-pushed
the
session-state-improvements
branch
from
7f69266 to
5ccc72c
Compare
| @@ -111,8 +111,9 @@ func newRedisCmdable(opts options.RedisStoreOptions) (Client, error) { | |||
| // Save takes a sessions.SessionState and stores the information from it | |||
| // to redies, and adds a new ticket cookie on the HTTP response writer | |||
There was a problem hiding this comment.
It'd be nice to get some tests written for this code, but I don't think it should block this change.
JoelSpeed
force-pushed
the
session-state-improvements
branch
from
5ccc72c to
6b5ac3e
Compare
|
@steakunderscore I have rebased as there was a small conflict. I agree, this is under tested, will make an issue to fix this (edit: there's a PR in progress to update the tests, it's not perfect but goes some of the way to getting testing there) |
JoelSpeed
force-pushed
the
session-state-improvements
branch
from
6b5ac3e to
1a0945e
Compare
Description
This is a follow on to #535 that refactors some of the session state code to try and make it easier to manage
Motivation and Context
Improve maintainability of the project
How Has This Been Tested?
Unit tests only
Checklist: