Introduce pkg/pagination #613
Conversation
jzelinskie
added
kind/cleanup
pkg/pagination/pagination.go
Outdated
| var key *fernet.Key | ||
| key, err = fernet.DecodeKey(keyString) | ||
| if err != nil { | ||
| err = errors.New("invalid pagination key string: must be 32-bit URL-safe base64") |
There was a problem hiding this comment.
Is this intended to be un-exported error?
There was a problem hiding this comment.
I'll export it. I also just noticed that it's 32 bytes not bits. Woops
pkg/pagination/pagination.go
Outdated
| func MustGenerateNewKey() Key { | ||
| var key fernet.Key | ||
| if err := key.Generate(); err != nil { | ||
| panic("pagination: failed to read enough randomness to generate a new key") |
There was a problem hiding this comment.
I understand that by current implementation, it'll only return error because of insufficient randomness. But will this error be the only error that key.Generate function will generate in the future?
There was a problem hiding this comment.
I went down the rabbit hole and talked to a bunch of the people working on Go crypto about this. I think it's safe to assume that fernet probably isn't going to change anything in this function and that any errors from crypto/rand.Read() should panic.
pkg/pagination/pagination.go
Outdated
| // UnmarshalToken decrypts a token using provided key and decodes the result | ||
| // into the provided interface. | ||
| func (k Key) UnmarshalToken(token string, v interface{}) error { | ||
| decoded, _ := fernet.DecodeKey(string(k)) |
There was a problem hiding this comment.
Do we really want to suppress the error here? Yes, it was not handled before. If there's a valid reason, I think it makes sense to comment it.
There was a problem hiding this comment.
If you only ever make Keys from the constructors, they'd always be valid.
However, I'll change this so that the Key type actually keeps the decoded bytes in memory rather than the string representation.
This change refactors a lot of the code dealing with pagination so that fernet implementation details do not leak. - Deletes database/pgsql/token - Introduces a pagination package - Renames idPageNumber to Page and add a constructor and method.
This change pulls as much pagination logic out of the database implementation as possible. Database implementations should now be able to marshal whatever state they need into opaque tokens with the utilities in the pagination package.
This change refactors a lot of the code dealing with pagination so that
fernet implementation details do not leak.