Cleanup SecurityHelpers #1401

Merged
merged 3 commits into from Jan 24, 2013

4 participants

@farmdawgnation
Lift Web Framework member

It looks like we had some duplicated code in SecurityHelpers, so I cleaned it up a bit.

Discussed here: https://groups.google.com/d/topic/liftweb/79fshdCWGjQ/discussion

One outstanding question is why we're doing base64 encodes instead of hex encodes. I'm probably missing something silly, but it did seem odd. The output seems right regardless.

Let me know if there are any issues. :)

@fmpwizard
Lift Web Framework member

Thanks!, I'll be rebasing to master later this week

@farmdawgnation
Lift Web Framework member

Sounds good, but I still have an outstanding question about why we're Base64 encoding md5 hashes. Is there any chance you or @dpp know the reasoning behind that? It seems odd. (In fact, I think it may be the cause of an issue you noticed at Elemica this weekend - wherein the results of md5 are not URL-safe).

@nafg
@Shadowfiend
Lift Web Framework member

David replied at https://groups.google.com/d/msg/liftweb/79fshdCWGjQ/QF6JVQ9f5K4J :

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

@fmpwizard fmpwizard merged commit 893da61 into lift:master Jan 24, 2013
@farmdawgnation farmdawgnation deleted the unknown repository branch Feb 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment