Don't mutate WebhooksHelper parameters #137
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `is_valid_webhook_event_signature` method would raise an exception if the passed in `signature_key` string was frozen, which it arguably always should be! Enabling frozen string literals in the test file causes most of the tests to fail. The underlying problem was a call to `force_encoding` which does not appear to be necessary; changing the string's encoding this way will not change the underlying bytes, and so should not have any effect on digest calculation. Since I could not find anything in the repository history or issues to explain why the encoding step was there, I removed it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
is_valid_webhook_event_signaturemethod would raise an exception if the passed insignature_keystring was frozen, which it arguably always should be! Enabling frozen string literals in the test file causes most of the tests to fail.The underlying problem was a call to
force_encodingwhich does not appear to be necessary; changing the string's encoding this way will not change the underlying bytes, and so should not have any effect on digest calculation.Since I could not find anything in the repository history or issues to explain why the encoding step was there, I removed it.