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.
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
feat!(middleware/session): re-write session middleware with handler #3016
feat!(middleware/session): re-write session middleware with handler #3016
Changes from 6 commits
6e76847
ac9a028
81f6789
28790cb
7ffae3d
68f2739
92e6877
239db00
0b93c5c
7cb4a6e
b4c8ea8
aafee92
cd91db4
c3b303f
75cffca
2731428
4f04291
ee193dc
01571cb
5f032d4
1a5a3d7
52e41a4
ed95d83
c6e1c34
8a5663a
46845e6
ba0e491
d08b686
8df7c81
508cf24
6ee953b
355b8f5
2f3f724
c08ddc1
56f6ce0
9e406f4
12b219a
6812fc4
28aad65
d4e607e
14c7a6c
a865ba5
eaedc6d
d2cf5b8
b479895
afab580
6c0bf25
ad337f8
280d539
40da2c0
3ad4bc9
ffac824
9f8c2d7
ecac9ce
e272082
b262a08
937a9b3
9ec2b30
684dc8a
05d30a4
ec5a698
13a1eb4
9d3b032
9762767
e59905f
7765ee5
8716c95
0e302e9
951691d
3ac9b68
bc95c6a
6bba849
281c0e1
3d88ece
3ddfeae
c3d3f0c
0e9a73e
a467236
6f35ff8
f3c4e8e
e41ee74
07092c8
84adbe1
f6440e2
eac16b6
7068a0e
87a6cb9
e5e5fd8
00b9e07
23e823b
ba38786
f77fa8f
b54c954
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 35 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L35
Check warning on line 41 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve test coverage for error handling paths
The static analysis tool has identified several lines that are not covered by tests, primarily in error handling paths. This lack of coverage could lead to undetected issues in error scenarios. Consider adding test cases that exercise these error paths:
Improving test coverage for these scenarios will enhance the reliability of the CSRF middleware and ensure that error handling behaves as expected in various situations.
Would you like assistance in generating test cases for these error handling paths?
Also applies to: 74-74, 78-78, 87-87, 93-93, 97-97
🧰 Tools
🪛 GitHub Check: codecov/patch
Check warning on line 61 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L61
Check warning on line 67 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L67
Check warning on line 71 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L71
Check warning on line 80 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L80
Check warning on line 86 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L86
Check warning on line 90 in middleware/csrf/session_manager.go
Codecov / codecov/patch
middleware/csrf/session_manager.go#L90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a unique context key to prevent collisions
The constant
key
is set to0
, which might collide with other context values in the Fiber context. It's safer to use a unique key to avoid potential conflicts with other middleware or handlers.Consider defining the key as a unique type or string:
Committable suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do this instead:
type contextKey int
const key contextKey = 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sixcolors, that solution works well! Using an unexported
contextKey
type with anint
helps avoid context key collisions. Thank you for addressing this.(_/)
( ^_^ )
/ \
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Check warning on line 47 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L45-L47
Check warning on line 57 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L56-L57
Check warning on line 60 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L59-L60
Check warning on line 66 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L64-L66
Check warning on line 73 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L70-L73
Check warning on line 76 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L76
Check warning on line 79 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L79
Check warning on line 91 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L87-L91
Check warning on line 96 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L96
Check warning on line 98 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L98
Check warning on line 106 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L103-L106
Check warning on line 108 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L108
Check warning on line 116 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L112-L116
Check warning on line 124 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L120-L124
Check warning on line 126 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L126
Check warning on line 131 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L129-L131
Check warning on line 134 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L133-L134
Check warning on line 137 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L137
Check warning on line 139 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L139
Check warning on line 144 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L142-L144
Check warning on line 147 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L146-L147
Check warning on line 152 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L150-L152
Check warning on line 156 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L154-L156
Check warning on line 160 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L159-L160
Check warning on line 164 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L163-L164
Check warning on line 169 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L167-L169
Check warning on line 173 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L171-L173
Check warning on line 178 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L176-L178
Check warning on line 183 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L181-L183
Check warning on line 186 in middleware/session/middleware.go
Codecov / codecov/patch
middleware/session/middleware.go#L185-L186