Fix CSRF middleware tests for fasthttp 1.62#3471
Conversation
ReneWerner87
requested review from
efectn,
gaby and
sixcolors
and removed request for
a team
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dependabot/go_modules/github.com/valyala/fasthttp-1.62.0 #3471 +/- ##
===========================================================================================
Coverage ? 84.32%
===========================================================================================
Files ? 120
Lines ? 12157
Branches ? 0
===========================================================================================
Hits ? 10251
Misses ? 1478
Partials ? 428
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 7e67eb4 | Previous: 895abd5 | Ratio |
|---|---|---|---|
Benchmark_Compress_Levels/Brotli_LevelBestCompression - B/op |
5 B/op |
0 B/op |
+∞ |
BenchmarkAppendMsgdata |
0.6225 ns/op 1606.33 MB/s 0 B/op 0 allocs/op |
0.3122 ns/op 3202.74 MB/s 0 B/op 0 allocs/op |
1.99 |
BenchmarkAppendMsgdata - ns/op |
0.6225 ns/op |
0.3122 ns/op |
1.99 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the CSRF middleware tests for fasthttp 1.62 by updating the test routines that handle cookie reading and request resets.
- Update of request reset logic: Separates header and body resets using ctx.Request.Header.Reset() and ctx.Request.ResetBody().
- Refactored cookie retrieval logic to use fasthttp.AcquireCookie for both CSRF and session cookies.
|
|
||
| // Without CSRF cookie | ||
| ctx.Request.Reset() | ||
| ctx.Request.Header.Reset() |
There was a problem hiding this comment.
[nitpick] Consider refactoring the repeated sequence of resetting the header and body into a helper function to improve test readability and reduce duplication.
sixcolors
left a comment
sixcolors
left a comment
There was a problem hiding this comment.
Looks good, thanks @ReneWerner87
ReneWerner87
merged commit 8d798ee
into
dependabot/go_modules/github.com/valyala/fasthttp-1.62.0
…3440) * build(deps): bump github.com/valyala/fasthttp from 1.60.0 to 1.62.0 Bumps [github.com/valyala/fasthttp](https://github.com/valyala/fasthttp) from 1.60.0 to 1.62.0. - [Release notes](https://github.com/valyala/fasthttp/releases) - [Commits](valyala/fasthttp@v1.60.0...v1.62.0) --- updated-dependencies: - dependency-name: github.com/valyala/fasthttp dependency-version: 1.62.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * Fix CSRF middleware tests for fasthttp 1.62 (#3471) Fix CSRF tests for fasthttp 1.62 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: RW <rene@gofiber.io>
ReneWerner87
deleted the
codex/unittests-nach-fasthttp-update-analysieren
branch
Summary
fasthttp.CookieTest_CSRF_DeleteTokenTesting
go test ./middleware/csrf -run Test_CSRF_DeleteToken -count=1go test ./...