Skip to content

Conversation

codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 30, 2025

PR Summary

Bug Fixes and Code Quality Improvements

Overview

This PR addresses multiple bugs, improves code quality, and enhances test coverage across various middleware components. It fixes several critical issues in the CORS middleware, improves security in the environment variables middleware, and corrects numerous spelling errors and variable naming conventions throughout the codebase.

Change Types

Type Description
Bugfix Fixed CORS middleware header handling and function calls
Bugfix Fixed invalid loop syntax in retry package
Enhancement Improved security in environment variables middleware
Enhancement Added Keys() method to session middleware
Refactor Corrected spelling errors and variable naming conventions
Test Added comprehensive test coverage for multiple middleware components

Affected Modules

Module / File Change Description
cors/cors.go Fixed Origin header case preservation and corrected function calls
retry/exponential_backoff.go Fixed invalid range-based loop syntax
envvar/envvar.go Security improvement to prevent accidental exposure of all environment variables
session/middleware.go Added new Keys() method for retrieving all session keys
binder.go, mapping.go Fixed spelling of 'convertable' to 'convertible' in error names and messages
ctx.go Improved Subdomains method with better edge case handling
listen.go Fixed spelling errors in TLS configuration comments
prefork.go Renamed variable from 'childs' to 'children'
Multiple test files Added comprehensive test coverage and moved tests into their respective packages

Breaking Changes

  • Environment variables middleware no longer exposes all variables by default when no specific variables are requested
  • Renamed error variable from ErrMapNotConvertable to ErrMapNotConvertible

Notes for Reviewers

  • The CORS middleware changes fix a critical issue with header handling
  • The retry package contains fixes for invalid loop syntax that would cause compilation errors
  • Test files have been reorganized to be in the same package as their implementation
  • The environment variables middleware security improvement changes default behavior

gaby and others added 15 commits June 5, 2025 09:36
* Fix Subdomains offset handling

* Update docs, add extra unit-tests

* update logic and tests

* update logic

* fix lint

* Update docs and tests

* Update ctx.go

* update docs

* update docs

* update docs

* add test case for port

* Update offset documentation
Bumps [github.com/gofiber/schema](https://github.com/gofiber/schema) from 1.4.0 to 1.5.0.
- [Release notes](https://github.com/gofiber/schema/releases)
- [Commits](gofiber/schema@v1.4.0...v1.5.0)

---
updated-dependencies:
- dependency-name: github.com/gofiber/schema
  dependency-version: 1.5.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.38.0 to 0.39.0.
- [Commits](golang/crypto@v0.38.0...v0.39.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-version: 0.39.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
chore: fix spelling mistakes across codebase
…lang.org/x/crypto-0.39.0

build(deps): bump golang.org/x/crypto from 0.38.0 to 0.39.0
…thub.com/gofiber/schema-1.5.0

build(deps): bump github.com/gofiber/schema from 1.4.0 to 1.5.0
* remove ExcludeVars option

* Simplify envvar export logic
…details and infrastructure considerations (gofiber#3509)

* docs: Enhance CORS middleware documentation with preflight request details and infrastructure considerations

* docs: Update CORS middleware documentation with infrastructure handling options and best practices

* docs: fix CORS middleware documentation markdown linting issues
* Improve CORS middleware

* 🐛 fix: Remove unused 'slices' import from CORS middleware

---------

Co-authored-by: Jason McNeil <[email protected]>
♻️ refactor: Update loop syntax for retry mechanism
* ✨ feat: Update session middleware add Keys method and update docs to match key type any

* ✨ test: Add /keys endpoint to set and retrieve multiple session keys

* ✨ test: Refactor /keys endpoint to trim whitespace and improve key handling in session tests
* test: move config tests into earlydata_test

* Fix lint issue
* Consolidate idempotency tests

* Fix lint issues

* Fix more lint issues

* Fix lint issue
* test: add HSTS coverage

* Update middleware/helmet/helmet_test.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update helmet_test.go

* Update helmet_test.go

* Revert test changes

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@@ -45,7 +45,7 @@ func (e *ExponentialBackoff) Retry(f func() error) error {
e.currentInterval = e.InitialInterval
}
var err error
for i := 0; i < e.MaxRetryCount; i++ {
for range e.MaxRetryCount {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Syntax Error: Invalid range expression.

The code uses 'for range e.MaxRetryCount' which is invalid Go syntax as range requires an iterable type (slice, array, map, string, or channel), not an integer.

Current Code (Diff):

- 	for range e.MaxRetryCount {
+ 	for i := 0; i < e.MaxRetryCount; i++ {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
for range e.MaxRetryCount {
for i := 0; i < e.MaxRetryCount; i++ {

@@ -107,7 +107,7 @@ func Test_ExponentialBackoff_Next(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
for i := 0; i < tt.expBackoff.MaxRetryCount; i++ {
for i := range tt.expBackoff.MaxRetryCount {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Compilation Error: Invalid range expression.

Using range with an integer (MaxRetryCount) will cause a compilation failure as range in Go requires a slice, array, string, map, or channel to iterate over.

Current Code (Diff):

- 			for i := range tt.expBackoff.MaxRetryCount {
+ 			for i := 0; i < tt.expBackoff.MaxRetryCount; i++ {
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
for i := range tt.expBackoff.MaxRetryCount {
for i := 0; i < tt.expBackoff.MaxRetryCount; i++ {


// Function to set Preflight CORS headers
func setPreflightHeaders(c fiber.Ctx, allowOrigin, maxAge string, cfg Config) {
setSimpleHeaders(c, allowOrigin, cfg)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Missing required parameter in function call.

The call to setSimpleHeaders is missing the required maxAge parameter which will cause a compilation error.

Current Code (Diff):

-	setSimpleHeaders(c, allowOrigin, cfg)
+	setSimpleHeaders(c, allowOrigin, maxAge, cfg)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
setSimpleHeaders(c, allowOrigin, cfg)
setSimpleHeaders(c, allowOrigin, maxAge, cfg)

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.

3 participants