Some cleanups to SecurityHelpers

[Matt Farmer]

So, the contribution guidelines indicate that Pull Requests need to be discussed here before being opened. Seeing as this was a cleanup, I decided to just implement the changes in a clone and then paste in a link to the diff to facilitate a discussion. If I should do something different for changes like this in the future, let me know. :)

Anyway, I did some cleanups to SecurityHelpers. Specifically, the code for generating a base64 string was duplicated in a few places in the file, so I converted those to calls to base64Encode. I also removed some unneeded parens that I found. Here's the diff: https://github.com/farmdawgnation/framework/compare/master...cleanup_security_helpers

Let me know if you guys want to changes, and if so I'll open a Pull Request.

Cheers!

[Matt Farmer]

So, I realized this morning when I was in the shower that something didn't make sense. We're base 64 encoding md5 and sha hashes to turn them into a string... don't most people use hex encoding for those?

[Diego Medina]

Looks good to me, just to make sure, your re-write doesn't change the the output, right?

Diego
Sent from my android cell

--
--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
 
 

[Matt Farmer]

I have no reason to believe it does, which is to say the all the unit tests pass.

[Richard Dallaway]

There looks to be unit tests covering the expected output from the methods that have changed. E.g., 

hexDigest256("hello".getBytes) must_== "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"

So should be good if they all pass.

Thanks

Richard

[Diego Medina]

Awesome then!

Diego
Sent from my android cell

[David Pollak]

On Sat, Jan 19, 2013 at 6:08 AM, Matt Farmer <ma...@frmr.me> wrote:

So, I realized this morning when I was in the shower that something didn't make sense. We're base 64 encoding md5 and sha hashes to turn them into a string... don't most people use hex encoding for those?

No matter what people use, please do not silently change the results of something. There may be code (like lot of stuff I've written) that relies on the base 64 encoding.

 

On Saturday, January 19, 2013 12:39:31 AM UTC-5, Matt Farmer wrote:

So, the contribution guidelines indicate that Pull Requests need to be discussed here before being opened. Seeing as this was a cleanup, I decided to just implement the changes in a clone and then paste in a link to the diff to facilitate a discussion. If I should do something different for changes like this in the future, let me know. :)

Anyway, I did some cleanups to SecurityHelpers. Specifically, the code for generating a base64 string was duplicated in a few places in the file, so I converted those to calls to base64Encode. I also removed some unneeded parens that I found. Here's the diff: https://github.com/farmdawgnation/framework/compare/master...cleanup_security_helpers

Let me know if you guys want to changes, and if so I'll open a Pull Request.

Cheers!

--

--
Lift, the simply functional web framework: http://liftweb.net
Code: http://github.com/lift
Discussion: http://groups.google.com/group/liftweb
Stuck? Help us help you: https://www.assembla.com/wiki/show/liftweb/Posting_example_code
 
 
 

--

Telegram, Simply Beautiful CMS https://telegr.am

Lift, the simply functional web framework http://liftweb.net

Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im

[Matt Farmer]

o.O Er.... I didn't silently change the results. I left the results as-is (unless I botched something in my branch). I was just trying to start a discussion about it.

[David Pollak]

On Mon, Jan 21, 2013 at 12:04 PM, Matt Farmer <ma...@frmr.me> wrote:

o.O Er.... I didn't silently change the results. I left the results as-is (unless I botched something in my branch). I was just trying to start a discussion about it.

No worries... just wanted to make sure return values don't change unless the type signatures change in a way that would cause existing code to fail to compile. ;-)

[Matt Farmer]

Completely, 100% valid. :)

But while we're on the topic, do you mind if I pick your brain as to why base64 was chosen over a hex-string representation?

[David Pollak]

On Mon, Jan 21, 2013 at 12:19 PM, Matt Farmer <ma...@frmr.me> wrote:

Completely, 100% valid. :)

But while we're on the topic, do you mind if I pick your brain as to why base64 was chosen over a hex-string representation?

It's shorter. The value isn't as important as the brevity of the representation.

[Matt Farmer]

That's logic I can get behind.

Thanks David!