Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed #35: incorrect hmac signature encoding #36

Merged
merged 2 commits into from
May 26, 2015
Merged

Conversation

kidtronnix
Copy link
Contributor

No description provided.

@cirpo
Copy link
Contributor

cirpo commented May 24, 2015

@smaxwellstewart thanks for the PR but I don't get why we should return the output of hash_hmac in a raw format. Could you please explain? thanks

@odino
Copy link
Contributor

odino commented May 25, 2015

@cirpo theres an open issue with a more detailed explanation

@smaxwellstewart the changes seem to be ok, but could you fix the tests that are now broken?

@cirpo
Copy link
Contributor

cirpo commented May 25, 2015

#35

@kidtronnix
Copy link
Contributor Author

I can add fixes for the tests for sure.

@kidtronnix kidtronnix changed the title fixed #36: incorrect hmac signature encoding fixed #35: incorrect hmac signature encoding May 26, 2015
@kidtronnix
Copy link
Contributor Author

Seems like it's all passing apart from HHVM. Anybody have any ideas why that test is faling? Does it matter?

@cirpo
Copy link
Contributor

cirpo commented May 26, 2015

@smaxwellstewart for HHVM it doesn't matter, thanks :)

@odino
Copy link
Contributor

odino commented May 26, 2015

all good! Im gonna publish a new major, since as soon as you roll the library out old clients using hmac will not work (as verification will not pass for tokens generated pre-fix)

odino added a commit that referenced this pull request May 26, 2015
fixed #35: incorrect hmac signature encoding
@odino odino merged commit 801192b into namshi:master May 26, 2015
@odino
Copy link
Contributor

odino commented May 26, 2015

tagged 5.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants