TwiN/go-away
1
0
Fork 0
Code Issues 8 Pull requests 2 Projects Releases 38 Packages Wiki Activity Actions

fix: data race in remove accents #80

Merged
rafax merged 2 commits from fix_data_race into master 2024-02-15 23:13:24 -05:00
Conversation 2 Commits 2 Files changed 2 +10 -4
rafax commented 2024-02-15 16:12:45 -05:00 (Migrated from github.com)
Copy link

This PR fixes a data race associated with reuse of removeAccentsTransformer - when IsProfane() or other methods are called from multiple goroutines (ex. in a HTTP Server handler), sanitization fails with slice bounds out of range errors.

The issue is fixed by using a new instance of removeAccentsTransformer for every call to removeAccents.

To reproduce, run the newly added BenchmarkIsProfaneConcurrently_WithAccents with

go test -race -benchmem -bench ^BenchmarkIsProfaneConcurrently_WithAccents$

(note - it will pass on the last commit of this branch that contains a fix).

Summary

Fix a data race in removeAccents, making ProfanityDetector safe for use from multiple goroutines.

Checklist

  • Tested and/or added tests to validate that the changes work as intended, if applicable.
  • Updated documentation in README.md, if applicable.
This PR fixes a data race associated with reuse of `removeAccentsTransformer` - when IsProfane() or other methods are called from multiple goroutines (ex. in a HTTP Server handler), sanitization fails with `slice bounds out of range` errors. The issue is fixed by using a new instance of `removeAccentsTransformer` for every call to `removeAccents`. To reproduce, run the newly added `BenchmarkIsProfaneConcurrently_WithAccents` with ``` go test -race -benchmem -bench ^BenchmarkIsProfaneConcurrently_WithAccents$ ``` (note - it will pass on the last commit of this branch that contains a fix). ## Summary Fix a data race in `removeAccents`, making `ProfanityDetector` safe for use from multiple goroutines. ## Checklist <!-- Replace [ ] by [X] if you have completed the item --> - [x] Tested and/or added tests to validate that the changes work as intended, if applicable. - [x] Updated documentation in `README.md`, if applicable.
codecov-commenter commented 2024-02-15 23:12:52 -05:00 (Migrated from github.com)
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f5715a1) 100.00% compared to head (b22822e) 100.00%.
Report is 2 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff            @@
##            master       #80   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          191       167   -24     
=========================================
- Hits           191       167   -24

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

## [Codecov](https://app.codecov.io/gh/TwiN/go-away/pull/80?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) Report All modified and coverable lines are covered by tests :white_check_mark: > Comparison is base [(`f5715a1`)](https://app.codecov.io/gh/TwiN/go-away/commit/f5715a1e19db47d953519e9f1a670a62e599050b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) 100.00% compared to head [(`b22822e`)](https://app.codecov.io/gh/TwiN/go-away/pull/80?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.) 100.00%. > Report is 2 commits behind head on master. :exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality. <details><summary>Additional details and impacted files</summary> ```diff @@ Coverage Diff @@ ## master #80 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 191 167 -24 ========================================= - Hits 191 167 -24 ``` </details> [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/TwiN/go-away/pull/80?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Chris+C.).
TwiN commented 2024-02-15 23:14:16 -05:00 (Migrated from github.com)
Copy link

@rafax Thank you for the contribution!

@rafax Thank you for the contribution!
👍 1
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TwiN/go-away!80
No description provided.