URL-safe Base64 for use in routes and params #11
Conversation
| # Not a base64 string | ||
| end | ||
|
|
||
| parse(decoded) |
There was a problem hiding this comment.
Inline the whole thing: parse Base64.urlsafe_decode64(gid) rescue ''
|
Added style clean up from @dhh. Apologies for forgetting to put it in a feature branch. |
|
Would be great if we could map == to something else ala https://github.com/cheerful/URLcrypt/blob/master/lib/URLcrypt.rb#L18 to get a prettier token. |
|
Also, no, we don't need to base64 a sgid. It's already ready to go as a token. |
| end | ||
|
|
||
| def parse_base64(gid) | ||
| parse Base64.urlsafe_decode64(gid) rescue '' |
There was a problem hiding this comment.
If Base64 decoding fails, can just return nil rather than trying to parse an empty string.
|
I made changes based on the comments. @dhh, I took a simpler route to cleaning up the padding equals signs. Let me know if there was specific reason you linked to that URLcrypt method. |
| def to_param | ||
| Base64.urlsafe_encode64(to_s).sub(/=+$/, '') | ||
| end | ||
|
|
There was a problem hiding this comment.
These are valid chars in the base64 encoding, so removing them will munge the data. If we don't want them in the output, we'll need to tweak the encoding or use something else.
There was a problem hiding this comment.
Ok, I looked at http://en.wikipedia.org/wiki/Base64 and it seemed ok since these ending equals signs are always padding to make blocks of 4 chars. I'll look at other solutions.
There was a problem hiding this comment.
Ah, it does look like = is the only unwanted char, and it is safe to remove if we manage the padding ourselves, as you do. Nice. I thought ? and & were present as well, but it appears not. So we should be good to go with urlsafe_encode64!
|
Think I got everything, awaiting review. |
|
|
||
| test 'to param' do | ||
| assert_equal 'Z2lkOi8vYmN4L1BlcnNvbi81--bd2dab1418d8577e10cf93f8ec055b4b61690755', @person_sgid.to_param | ||
| end |
There was a problem hiding this comment.
Think a little clearer to test assert_equal @person_sgid.to_s, @person_sgid.to_param
We just care that to_param is the string representation.
There was a problem hiding this comment.
I made this change
|
Nice! Could you rebase & squash your commits? |
|
Cool, I've squashed to a single commit. |
URL-safe Base64 for use in routes and params
|
Thank you @bencrouse ! |
|
Glad to help! |
Two questions:
Base64.urlsafe_encode64instead because that seemed to make sense, that ok?