🧹 chore: Replace strings.TrimSpace with utils.TrimSpace #3918
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR systematically refactors whitespace trimming across the codebase by replacing ad-hoc trim implementations with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @gaby, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code consistency and clarity by migrating all relevant string trimming operations to use the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
gaby
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3918 +/- ##
==========================================
- Coverage 91.66% 91.64% -0.02%
==========================================
Files 119 119
Lines 10096 10096
==========================================
- Hits 9254 9252 -2
- Misses 534 535 +1
- Partials 308 309 +1
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:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the codebase to consistently use utils.TrimSpace for trimming whitespace, which is a good improvement for code consistency. The changes are mostly correct, but I've identified one instance in helpers.go where the new function is used with an incorrect type, which will lead to a compilation error. I've provided a specific comment and a code suggestion to fix this issue. The documentation update is also a nice improvement.
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to consistently use utils.TrimSpace() instead of utils.Trim(x, ' ') and strings.TrimSpace(), and fixes a documentation example to avoid variable shadowing.
- Standardizes whitespace trimming to use
utils.TrimSpace()across the entire codebase - Updates the v3 migration example in documentation to use correct function calls and avoid shadowing the
svariable
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| req.go | Replaced multiple utils.Trim(x, ' ') calls with utils.TrimSpace(x) in request parsing functions |
| middleware/session/middleware_test.go | Changed strings.TrimSpace to utils.TrimSpace for consistency |
| middleware/csrf/csrf_test.go | Updated two instances of utils.Trim to utils.TrimSpace in cookie parsing tests |
| middleware/csrf/csrf.go | Replaced utils.Trim with utils.TrimSpace when processing trusted origins |
| middleware/cors/cors.go | Updated origin trimming to use utils.TrimSpace |
| middleware/compress/compress.go | Changed token comparison to use utils.TrimSpace |
| middleware/cache/cache.go | Standardized both utils.Trim and strings.TrimSpace to utils.TrimSpace |
| helpers.go | Updated multiple instances in helper functions to use utils.TrimSpace |
| docs/whats_new.md | Fixed v3 migration example to use utils.TrimSpace instead of utils.Trim and renamed s to str to avoid shadowing |
| ctx_test.go | Changed strings.TrimSpace to utils.TrimSpace in compression tests |
| ctx.go | Updated transfer encoding and WebSocket checks to use utils.TrimSpace |
| client/response.go | Replaced utils.Trim with utils.TrimSpace in String() method |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
middleware/cache/cache.go (1)
426-459: Cache-Control parsing now consistently usesutils.TrimSpaceNormalizing each directive with
utils.TrimSpace(utils.ToLower(part))in bothparseMaxAgeandallowsSharedCachekeeps Cache-Control handling case-insensitive and whitespace-tolerant while aligning with utils-based helpers. If you touch this again, you could optionally factor that normalization into a tiny helper to avoid repeating the same line in both functions, but it's not critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
client/response.go(1 hunks)ctx.go(2 hunks)ctx_test.go(2 hunks)docs/whats_new.md(1 hunks)helpers.go(3 hunks)middleware/cache/cache.go(2 hunks)middleware/compress/compress.go(1 hunks)middleware/cors/cors.go(1 hunks)middleware/csrf/csrf.go(1 hunks)middleware/csrf/csrf_test.go(2 hunks)middleware/session/middleware_test.go(2 hunks)req.go(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Files:
middleware/csrf/csrf.gomiddleware/cors/cors.gomiddleware/compress/compress.goctx.goreq.gomiddleware/session/middleware_test.goclient/response.gomiddleware/csrf/csrf_test.gomiddleware/cache/cache.gohelpers.goctx_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
When adding Go tests, always invoke
t.Parallel()at the start of each test and subtest to maximize concurrency
Files:
middleware/session/middleware_test.gomiddleware/csrf/csrf_test.goctx_test.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/whats_new.md
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
make markdownto lint all Markdown files when modifying code
Files:
docs/whats_new.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: gofiber/fiber PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T13:34:08.100Z
Learning: Applies to **/*.go : Prefer `github.com/gofiber/utils/v2` helpers (for example, `utils.Trim`) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
📚 Learning: 2025-11-26T13:34:08.100Z
Learnt from: CR
Repo: gofiber/fiber PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T13:34:08.100Z
Learning: Applies to **/*.go : Prefer `github.com/gofiber/utils/v2` helpers (for example, `utils.Trim`) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Applied to files:
middleware/cors/cors.goctx.goreq.gomiddleware/session/middleware_test.goclient/response.gomiddleware/cache/cache.gohelpers.goctx_test.godocs/whats_new.md
📚 Learning: 2025-09-28T17:39:26.644Z
Learnt from: arturmelanchyk
Repo: gofiber/fiber PR: 3768
File: client/request.go:162-166
Timestamp: 2025-09-28T17:39:26.644Z
Learning: In fasthttp-based code like gofiber, using utils.UnsafeString(key) instead of string(key) is acceptable when values are used immediately within the same call stack and not stored for later use, as it avoids allocations and aligns with fasthttp's zero-allocation design philosophy.
Applied to files:
middleware/session/middleware_test.godocs/whats_new.md
📚 Learning: 2024-11-15T07:56:21.623Z
Learnt from: ReneWerner87
Repo: gofiber/fiber PR: 3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2025-10-16T07:19:52.418Z
Learnt from: grivera64
Repo: gofiber/fiber PR: 3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:19:52.418Z
Learning: In the Fiber codebase, the linter does not allow `require` assertions from within HTTP handlers (including net/http-style handlers). Use `t.Fatalf`, `t.Errorf`, or similar `testing.T` methods for error handling inside handler functions instead.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/store.go:164-167
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests in this project use testify require.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2025-10-16T07:15:26.529Z
Learnt from: grivera64
Repo: gofiber/fiber PR: 3807
File: adapter_test.go:118-144
Timestamp: 2025-10-16T07:15:26.529Z
Learning: In Fiber v3, net/http handlers (http.Handler, http.HandlerFunc, or raw func(http.ResponseWriter, *http.Request)) can be passed directly to routing methods like app.Get(), app.Post(), etc. The framework automatically detects and wraps them internally via toFiberHandler/collectHandlers. The github.com/gofiber/fiber/v3/middleware/adaptor package is legacy and should not be suggested for tests or code using native net/http handler support.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2024-07-26T21:00:12.902Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 2922
File: middleware/cors/utils.go:63-71
Timestamp: 2024-07-26T21:00:12.902Z
Learning: The project uses the testify/assert package for assertions in unit tests.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2024-11-10T23:44:13.704Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3193
File: middleware/adaptor/adaptor.go:111-111
Timestamp: 2024-11-10T23:44:13.704Z
Learning: In the `middleware/adaptor/adaptor.go` file of the Fiber framework, when updating context handling, replacing `c.Context()` with `c.RequestCtx()` is appropriate to access the `fasthttp.RequestCtx`.
Applied to files:
middleware/session/middleware_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
middleware/csrf/csrf_test.go
📚 Learning: 2024-09-25T17:08:07.693Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:164-165
Timestamp: 2024-09-25T17:08:07.693Z
Learning: In the `Test_CSRF_WithSession_Middleware` function, calling `session.NewWithStore()` without arguments is acceptable, as the default configuration is sufficient.
Applied to files:
middleware/csrf/csrf_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
middleware/csrf/csrf_test.go
📚 Learning: 2024-06-30T00:38:06.580Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3051
File: middleware/session/session.go:215-216
Timestamp: 2024-06-30T00:38:06.580Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Applied to files:
middleware/csrf/csrf_test.go
🧬 Code graph analysis (1)
req.go (2)
constants.go (1)
HeaderRange(236-236)error.go (1)
ErrRangeMalformed(32-32)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Agent
- GitHub Check: lint
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (13)
middleware/cors/cors.go (1)
51-74: LGTM! Improved whitespace handling in origin validation.The change from
utils.Trim(origin, ' ')toutils.TrimSpace(origin)is appropriate and improves robustness by handling all Unicode whitespace characters (tabs, newlines, etc.) rather than just ASCII spaces. This aligns with the coding guideline to prefergithub.com/gofiber/utils/v2helpers.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
middleware/session/middleware_test.go (1)
127-143: LGTM! Consistent use of utils helpers in test code.The migration from
strings.TrimSpacetoutils.TrimSpacefollows the project's coding guideline to prefergithub.com/gofiber/utils/v2helpers. The import was correctly added, and the usage in the comma-separated key parsing is appropriate.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
middleware/csrf/csrf.go (1)
69-91: LGTM! Consistent whitespace normalization for trusted origins.The change from
utils.Trim(origin, ' ')toutils.TrimSpace(origin)is consistent with the CORS middleware (middleware/cors/cors.go line 57) and provides more robust whitespace handling for origin validation. This standardizes the approach across the codebase.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
middleware/csrf/csrf_test.go (2)
306-311: LGTM! Test updated to use utils.TrimSpace for header parsing.The test code correctly adopts
utils.TrimSpacefor parsing Set-Cookie headers, maintaining consistency with the production code changes. This is appropriate for HTTP header parsing where various whitespace characters may be present.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
478-483: LGTM! Consistent test update for Set-Cookie parsing.This change mirrors the update at line 307, ensuring both CSRF session tests use the same whitespace trimming approach. The consistency is good for maintainability.
client/response.go (1)
92-95: LGTM! Enhanced response body trimming.The change from
utils.Trim(string(r.Body()), ' ')toutils.TrimSpace(string(r.Body()))improves the robustness of the String() method by handling all types of whitespace characters (newlines, tabs, carriage returns, etc.) that may appear at the start or end of HTTP response bodies.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
middleware/compress/compress.go (1)
12-19: LGTM! Improved token parsing for HTTP headers.The change from
utils.Trim(part, ' ')toutils.TrimSpace(part)enhances thehasTokenfunction to properly handle all whitespace characters in HTTP header values. This is consistent with the broader PR changes to standardize whitespace trimming across header parsing logic.As per coding guidelines, preferring utils/v2 helpers for string manipulation.
ctx_test.go (1)
711-711: Encoding token trimming change looks goodUsing
utils.TrimSpace(enc)in both compression tests keeps the intent (trimming leading/trailing whitespace from eachContent-Encodingtoken) while aligning with the project’s preference forutilshelpers. No further changes needed. As per coding guidelines, this is the preferred helper for string manipulation.Also applies to: 945-945
helpers.go (1)
539-539: Standardizing whitespace trimming with utils.TrimSpace is appropriateSwitching to
utils.TrimSpaceingetOffer,isEtagStale, andparseAddrunifies whitespace handling, likely broadening support beyond plain spaces without affecting correctness, and follows the repo’s guidance to preferutilshelpers. As per coding guidelines, this is the desired direction.Also applies to: 663-663, 698-698
docs/whats_new.md (1)
2218-2229: Updated utils example looks correct; remember markdown lintingThe switch to
utils.TrimSpaceandutils.ToStringplus thestrrename is consistent with the v3 utils guidance and avoids variable shadowing. Please also runmake markdownso markdown lint stays green for this doc change.req.go (2)
487-501: Content-Type normalization viautils.TrimSpaceis appropriateTrimming
ctwithutils.TrimSpacebefore the comparison makes theIscheck more robust to stray whitespace while staying consistent with utils usage.
741-816: Range header parsing whitespace handling looks solidNormalizing
Rangewithutils.TrimSpace(for the full header, type, ranges, and per-range bounds) improves tolerance of optional whitespace without changing the core parsing or validation rules (bytestype check, bounds viautils.ParseUint, malformed/error paths). This aligns with the broader move toutils.TrimSpace.ctx.go (1)
385-424: Transfer-Encoding and WebSocket upgrade trimming changes are consistentUsing
utils.TrimSpaceforTransfer-Encodingtokens and theConnection: upgradevalue keeps the logic correct while standardizing on the utils helpers for whitespace normalization and case-insensitive parsing. No functional regressions stand out.Also applies to: 426-440
ReneWerner87
deleted the
replace-strings.trimspace-with-utils.trimspace
branch
Summary