Skip to content

fix: RequestValidator thread-safety#1

Merged
Swimburger merged 4 commits intoSwimburger:optimize-request-validationfrom
benmccallum:patch-1
Jan 3, 2023
Merged

fix: RequestValidator thread-safety#1
Swimburger merged 4 commits intoSwimburger:optimize-request-validationfrom
benmccallum:patch-1

Conversation

@Swimburger
Copy link
Copy Markdown
Owner

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

Fixes twilio#466

Fixes

Makes RequestValidator thread-safe by not holding on to instances of the crypto algos used and instead new'ing them up and disposing of them per validation.

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@Swimburger Swimburger changed the base branch from main to optimize-request-validation December 31, 2022 01:01
@Swimburger
Copy link
Copy Markdown
Owner Author

Without modifications, this code improves the performance but slightly increases the allocations for the unhappy path.
With minor modifications, reusing the same HMACSHA1 for both calls to GetValidationSignature the performances is up and allocations down for happy and unhappy paths.

Method Mean Error StdDev Gen0 Gen1 Allocated
OriginalUnhappyPath 15.204 us 0.1571 us 0.1226 us 1.9836 0.2441 12.27 KB
CurrentUnhappyPath 11.855 us 0.2103 us 0.3148 us 1.4191 - 8.75 KB
OriginalHappyPath 9.241 us 0.1789 us 0.1757 us 1.6327 0.2289 10.02 KB
CurrentHappyPath 3.135 us 0.0627 us 0.0586 us 0.6676 0.0038 4.09 KB

Swimburger added a commit that referenced this pull request Dec 31, 2022
@Swimburger
Copy link
Copy Markdown
Owner Author

Using the mentioned one-shot alternative for .NET 6 and up (HMACSHA1.HashData) further improves efficiency:

Method Mean Error StdDev Gen0 Gen1 Allocated
OriginalUnhappyPath 14.902 us 0.0598 us 0.0467 us 1.9836 0.2441 12.27 KB
CurrentUnhappyPath 10.830 us 0.0574 us 0.0509 us 1.3885 - 8.52 KB
OriginalHappyPath 8.756 us 0.1750 us 0.1551 us 1.6327 0.2289 10.02 KB
CurrentHappyPath 2.625 us 0.0239 us 0.0199 us 0.6256 - 3.85 KB

Tested on optimize-request-validation-thread-safe branch.

@Swimburger Swimburger merged commit 709f8b8 into Swimburger:optimize-request-validation Jan 3, 2023
@benmccallum benmccallum deleted the patch-1 branch January 12, 2023 23:43
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.

Possible thread safety issue with RequestValidator.cs causing System.Security.Cryptography.CryptographicException

2 participants