From d305cc4ca5db64c1ab87bfa0947e0f25a568ee2a Mon Sep 17 00:00:00 2001 From: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> Date: Wed, 7 Feb 2024 08:12:51 -0500 Subject: [PATCH 1/9] Add go1.22 to test matrix (#2835) --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8ece46db31..2506993d71 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: Build: strategy: matrix: - go-version: [1.20.x, 1.21.x] + go-version: [1.20.x, 1.21.x, 1.22.x] platform: [ubuntu-latest, windows-latest, macos-latest] runs-on: ${{ matrix.platform }} steps: From 847a4a959d979c46c3a0afd47409929bd158a7f0 Mon Sep 17 00:00:00 2001 From: Lino Gomez Date: Wed, 7 Feb 2024 22:58:12 -0800 Subject: [PATCH 2/9] Fix: Typo in routing.md (#2836) fix routing.md typo --- docs/guide/routing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/routing.md b/docs/guide/routing.md index 70d746a317..b8ab756694 100644 --- a/docs/guide/routing.md +++ b/docs/guide/routing.md @@ -145,7 +145,7 @@ We have adapted the routing strongly to the express routing, but currently witho Route constraints execute when a match has occurred to the incoming URL and the URL path is tokenized into route values by parameters. The feature was intorduced in `v2.37.0` and inspired by [.NET Core](https://docs.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-6.0#route-constraints). :::caution -Constraints aren't validation for parameters. If constraint aren't valid for parameter value, Fiber returns **404 handler**. +Constraints aren't validation for parameters. If constraints aren't valid for a parameter value, Fiber returns **404 handler**. ::: | Constraint | Example | Example matches | From 2b03f47fae08c6d610f8f595a3e42f0a8d8c658b Mon Sep 17 00:00:00 2001 From: Joey Date: Fri, 9 Feb 2024 12:27:21 +0100 Subject: [PATCH 3/9] =?UTF-8?q?=F0=9F=9A=80=20Performance=20improvements?= =?UTF-8?q?=20(#2838)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add new supporter * Add new test condition * Add Handler Type * Update app.go * Update group.go * Add Handler Type * Update ViewEngine * Update Templates Interface * Update template examples * Update fasthttp to v1.13.1 * Default cookie SameSite to Lax * - static file routing fixed for fasthttp 1.13 - fix expected cookie values in tests * Update template examples * Update fasthttp to v1.13.1 Co-Authored-By: Thomas van Vugt * Cookie SameSite defaults to Lax Co-Authored-By: Thomas van Vugt Co-Authored-By: Queru * Fix router bug Co-Authored-By: RW * Remove unused code Co-Authored-By: RW * Add more static tests Co-Authored-By: RW * Update app_test.go Co-Authored-By: RW * Update Static tests Co-Authored-By: RW * Update app_test.go Co-Authored-By: RW * Update app_test.go Co-Authored-By: RW * Fix handler next calls Co-Authored-By: RW * Update router.go Co-Authored-By: RW * Update ctx.go Co-Authored-By: RW * Update app_test.go Co-Authored-By: RW * Remove nextHandler Co-Authored-By: RW * Remove lencount Co-Authored-By: RW * Add ErrorHandler * Add ErrorHandler tests * Add recover by default * Enable recover by default * Add App() * Add ErrorHandler * Enable recover by default * Add ErrorHandler * Add App() & Middleware * Add RequestID * Add new supporters * Update shields * Add mw * Update basic_auth.go * Update README.md * Update spacing * Update basic_auth_test.go * Update ctx_test.go * Add tests * Update middleware * up * Small improvements Use optimized `utils.ToString` and avoid `once.Do` **Before** ``` BenchmarkLogfKeyAndValues/test_logf_with_debug_level_and_key-values-24 7323432 153.8 ns/op 89 B/op 1 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_info_level_and_key-values-24 8171703 144.5 ns/op 81 B/op 1 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_warn_level_and_key-values-24 8207860 142.8 ns/op 81 B/op 1 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_format_and_key-values-24 7500332 159.1 ns/op 135 B/op 2 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_one_key-24 10024760 131.0 ns/op 155 B/op 2 allocs/op ``` **After** ``` BenchmarkLogfKeyAndValues/test_logf_with_debug_level_and_key-values-24 13797813 77.42 ns/op 77 B/op 0 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_info_level_and_key-values-24 15375350 75.43 ns/op 73 B/op 1 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_warn_level_and_key-values-24 14926300 75.28 ns/op 75 B/op 1 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_format_and_key-values-24 12860275 90.27 ns/op 134 B/op 2 allocs/op BenchmarkLogfKeyAndValues/test_logf_with_one_key-24 15649615 74.98 ns/op 100 B/op 1 allocs/op ``` * Fix WithCtxCaller test * Fix lint * Fix lint * Replace Sprintf with byebufferpool in ctx.String() # Original fn using Sprintf Benchmark_Ctx_String-24 3846717 318.0 ns/op 152 B/op 8 allocs/op Benchmark_Ctx_String-24 3780208 315.9 ns/op 152 B/op 8 allocs/op Benchmark_Ctx_String-24 3627513 315.1 ns/op 152 B/op 8 allocs/op Benchmark_Ctx_String-24 3712863 317.4 ns/op 152 B/op 8 allocs/op // Modified using bytebufferpool Benchmark_Ctx_String-24 8131666 149.3 ns/op 96 B/op 5 allocs/op Benchmark_Ctx_String-24 7626406 148.3 ns/op 96 B/op 5 allocs/op Benchmark_Ctx_String-24 8194621 149.2 ns/op 96 B/op 5 allocs/op Benchmark_Ctx_String-24 8297750 156.6 ns/op 96 B/op 5 allocs/op * Fix linting * Use bytebufferpool in default logger * Fix linting * Lint fix * Update linter.yml * Update linter.yml * Disable caching as recommended by golangci-lint * 🩹 fix lint errors --------- Co-authored-by: ReneWerner87 Co-authored-by: Thomas van Vugt Co-authored-by: Queru Co-authored-by: ReneWerner87 Co-authored-by: Juan Calderon-Perez <835733+gaby@users.noreply.github.com> --- .github/workflows/linter.yml | 7 ++++ ctx.go | 41 ++++++++++++++++----- ctx_test.go | 14 ++++++++ log/default.go | 16 ++++----- log/default_test.go | 56 ++++++++++++++++++++++++++++- middleware/logger/default_logger.go | 56 +++++++++++++++++++++++------ 6 files changed, 160 insertions(+), 30 deletions(-) diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index 21016895a4..4b2fba5888 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -7,8 +7,14 @@ on: - master - main pull_request: + permissions: + # Required: allow read access to the content for analysis. contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + pull-requests: read + # Optional: Allow write access to checks to allow the action to annotate code in the PR. + checks: write jobs: golangci: @@ -21,6 +27,7 @@ jobs: with: # NOTE: Keep this in sync with the version from go.mod go-version: "1.20.x" + cache: false - name: golangci-lint uses: golangci/golangci-lint-action@v3 diff --git a/ctx.go b/ctx.go index 9257825e7b..d5d847f977 100644 --- a/ctx.go +++ b/ctx.go @@ -1605,14 +1605,39 @@ func (c *DefaultCtx) Status(status int) Ctx { // // The returned value may be useful for logging. func (c *DefaultCtx) String() string { - return fmt.Sprintf( - "#%016X - %s <-> %s - %s %s", - c.fasthttp.ID(), - c.fasthttp.LocalAddr(), - c.fasthttp.RemoteAddr(), - c.fasthttp.Request.Header.Method(), - c.fasthttp.URI().FullURI(), - ) + // Get buffer from pool + buf := bytebufferpool.Get() + + // Start with the ID, converting it to a hex string without fmt.Sprintf + buf.WriteByte('#') //nolint:errcheck // It is fine to ignore the error + // Convert ID to hexadecimal + id := strconv.FormatUint(c.fasthttp.ID(), 16) + // Pad with leading zeros to ensure 16 characters + for i := 0; i < (16 - len(id)); i++ { + buf.WriteByte('0') //nolint:errcheck // It is fine to ignore the error + } + buf.WriteString(id) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" - ") //nolint:errcheck // It is fine to ignore the error + + // Add local and remote addresses directly + buf.WriteString(c.fasthttp.LocalAddr().String()) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" <-> ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(c.fasthttp.RemoteAddr().String()) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" - ") //nolint:errcheck // It is fine to ignore the error + + // Add method and URI + buf.Write(c.fasthttp.Request.Header.Method()) //nolint:errcheck // It is fine to ignore the error + buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + buf.Write(c.fasthttp.URI().FullURI()) //nolint:errcheck // It is fine to ignore the error + + // Allocate string + str := buf.String() + + // Reset buffer + buf.Reset() + bytebufferpool.Put(buf) + + return str } // Type sets the Content-Type HTTP header to the MIME type specified by the file extension. diff --git a/ctx_test.go b/ctx_test.go index df896920e3..93b1a1d4cf 100644 --- a/ctx_test.go +++ b/ctx_test.go @@ -4631,6 +4631,20 @@ func Test_Ctx_String(t *testing.T) { require.Equal(t, "#0000000000000000 - 0.0.0.0:0 <-> 0.0.0.0:0 - GET http:///", c.String()) } +// go test -v -run=^$ -bench=Benchmark_Ctx_String -benchmem -count=4 +func Benchmark_Ctx_String(b *testing.B) { + var str string + app := New() + ctx := app.NewCtx(&fasthttp.RequestCtx{}) + b.ReportAllocs() + b.ResetTimer() + + for n := 0; n < b.N; n++ { + str = ctx.String() + } + require.Equal(b, "#0000000000000000 - 0.0.0.0:0 <-> 0.0.0.0:0 - GET http:///", str) +} + func TestCtx_ParamsInt(t *testing.T) { // Create a test context and set some strings (or params) // create a fake app to be used within this test diff --git a/log/default.go b/log/default.go index abc9c8f4d7..96c751a940 100644 --- a/log/default.go +++ b/log/default.go @@ -6,8 +6,8 @@ import ( "io" "log" "os" - "sync" + "github.com/gofiber/utils/v2" "github.com/valyala/bytebufferpool" ) @@ -75,8 +75,6 @@ func (l *defaultLogger) privateLogw(lv Level, format string, keysAndValues []any if format != "" { _, _ = buf.WriteString(format) //nolint:errcheck // It is fine to ignore the error } - var once sync.Once - isFirst := true // Write keys and values privateLog buffer if len(keysAndValues) > 0 { if (len(keysAndValues) & 1) == 1 { @@ -84,14 +82,12 @@ func (l *defaultLogger) privateLogw(lv Level, format string, keysAndValues []any } for i := 0; i < len(keysAndValues); i += 2 { - if format == "" && isFirst { - once.Do(func() { - _, _ = fmt.Fprintf(buf, "%s=%v", keysAndValues[i], keysAndValues[i+1]) - isFirst = false - }) - continue + if i > 0 || format != "" { + _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error } - _, _ = fmt.Fprintf(buf, " %s=%v", keysAndValues[i], keysAndValues[i+1]) + _, _ = buf.WriteString(keysAndValues[i].(string)) //nolint:errcheck // It is fine to ignore the error + _ = buf.WriteByte('=') //nolint:errcheck // It is fine to ignore the error + _, _ = buf.WriteString(utils.ToString(keysAndValues[i+1])) //nolint:errcheck // It is fine to ignore the error } } diff --git a/log/default_test.go b/log/default_test.go index 78ef0204ca..3b1eb6f06b 100644 --- a/log/default_test.go +++ b/log/default_test.go @@ -156,6 +156,60 @@ func Test_LogfKeyAndValues(t *testing.T) { } } +func BenchmarkLogfKeyAndValues(b *testing.B) { + tests := []struct { + name string + level Level + format string + keysAndValues []any + }{ + { + name: "test logf with debug level and key-values", + level: LevelDebug, + format: "", + keysAndValues: []any{"name", "Bob", "age", 30}, + }, + { + name: "test logf with info level and key-values", + level: LevelInfo, + format: "", + keysAndValues: []any{"status", "ok", "code", 200}, + }, + { + name: "test logf with warn level and key-values", + level: LevelWarn, + format: "", + keysAndValues: []any{"error", "not found", "id", 123}, + }, + { + name: "test logf with format and key-values", + level: LevelWarn, + format: "test", + keysAndValues: []any{"error", "not found", "id", 123}, + }, + { + name: "test logf with one key", + level: LevelWarn, + format: "", + keysAndValues: []any{"error"}, + }, + } + + for _, tt := range tests { + b.Run(tt.name, func(b *testing.B) { + var buf bytes.Buffer + l := &defaultLogger{ + stdlog: log.New(&buf, "", 0), + level: tt.level, + depth: 4, + } + for i := 0; i < b.N; i++ { + l.privateLogw(tt.level, tt.format, tt.keysAndValues) + } + }) + } +} + func Test_WithContextCaller(t *testing.T) { logger = &defaultLogger{ stdlog: log.New(os.Stderr, "", log.Lshortfile), @@ -169,7 +223,7 @@ func Test_WithContextCaller(t *testing.T) { WithContext(ctx).Info("") Info("") - require.Equal(t, "default_test.go:169: [Info] \ndefault_test.go:170: [Info] \n", string(w.b)) + require.Equal(t, "default_test.go:223: [Info] \ndefault_test.go:224: [Info] \n", string(w.b)) } func Test_SetLevel(t *testing.T) { diff --git a/middleware/logger/default_logger.go b/middleware/logger/default_logger.go index 26669e3c16..49a9202d93 100644 --- a/middleware/logger/default_logger.go +++ b/middleware/logger/default_logger.go @@ -4,6 +4,7 @@ import ( "fmt" "io" "os" + "strconv" "sync" "github.com/gofiber/fiber/v3" @@ -47,17 +48,50 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { if data.ChainErr != nil { formatErr = " | " + data.ChainErr.Error() } - _, _ = buf.WriteString( //nolint:errcheck // This will never fail - fmt.Sprintf("%s | %3d | %13v | %15s | %-7s | %-"+data.ErrPaddingStr+"s %s\n", - data.Timestamp.Load().(string), - c.Response().StatusCode(), - data.Stop.Sub(data.Start), - c.IP(), - c.Method(), - c.Path(), - formatErr, - ), - ) + + // Helper function to append fixed-width string with padding + fixedWidth := func(s string, width int, rightAlign bool) { + if rightAlign { + for i := len(s); i < width; i++ { + _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + } + _, _ = buf.WriteString(s) //nolint:errcheck // It is fine to ignore the error + } else { + _, _ = buf.WriteString(s) //nolint:errcheck // It is fine to ignore the error + for i := len(s); i < width; i++ { + _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + } + } + } + + // Timestamp + _, _ = buf.WriteString(data.Timestamp.Load().(string)) //nolint:errcheck // It is fine to ignore the error + _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + + // Status Code with 3 fixed width, right aligned + fixedWidth(strconv.Itoa(c.Response().StatusCode()), 3, true) + _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + + // Duration with 13 fixed width, right aligned + fixedWidth(data.Stop.Sub(data.Start).String(), 13, true) + _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + + // Client IP with 15 fixed width, right aligned + fixedWidth(c.IP(), 15, true) + _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + + // HTTP Method with 7 fixed width, left aligned + fixedWidth(c.Method(), 7, false) + _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + + // Path with dynamic padding for error message, left aligned + errPadding, _ := strconv.Atoi(data.ErrPaddingStr) //nolint:errcheck // It is fine to ignore the error + fixedWidth(c.Path(), errPadding, false) + + // Error message + _, _ = buf.WriteString(" ") //nolint:errcheck // It is fine to ignore the error + _, _ = buf.WriteString(formatErr) //nolint:errcheck // It is fine to ignore the error + _, _ = buf.WriteString("\n") //nolint:errcheck // It is fine to ignore the error } // Write buffer to output From 059c0e33ed8ad5b7094f8045f9ad772a3ec20399 Mon Sep 17 00:00:00 2001 From: Nicholas Jackson Date: Fri, 9 Feb 2024 12:23:59 -0800 Subject: [PATCH 4/9] =?UTF-8?q?=F0=9F=8E=A8=20Style:=20Clean=20up=20errche?= =?UTF-8?q?ck=20config?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Globally ignore several methods that always return nil error. Disable revive and gosec rules for error checking in favor of errcheck. --- .golangci.yml | 10 ++++++++- ctx.go | 30 ++++++++++++------------- ctx_test.go | 4 ++-- log/default.go | 18 +++++++-------- log/default_test.go | 4 ++-- middleware/etag/etag.go | 8 +++---- middleware/logger/default_logger.go | 34 ++++++++++++++--------------- middleware/session/store.go | 2 +- redirect.go | 12 +++++----- redirect_test.go | 15 ++++++++++--- 10 files changed, 77 insertions(+), 60 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5d40f30fcd..c19f219e77 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -15,6 +15,11 @@ linters-settings: check-type-assertions: true check-blank: true disable-default-exclusions: true + exclude-functions: + - '(*bytes.Buffer).Write' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).Write' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteByte' # always returns nil error + - '(*github.com/valyala/bytebufferpool.ByteBuffer).WriteString' # always returns nil error errchkjson: report-no-exported: true @@ -40,7 +45,7 @@ linters-settings: gosec: excludes: - - G104 + - G104 # Provided by errcheck config: global: audit: true @@ -124,6 +129,9 @@ linters-settings: disabled: true - name: unchecked-type-assertion disabled: true # TODO https://github.com/gofiber/fiber/issues/2816 + # Provided by errcheck + - name: unhandled-error + disabled: true stylecheck: checks: diff --git a/ctx.go b/ctx.go index d5d847f977..d58ef263b7 100644 --- a/ctx.go +++ b/ctx.go @@ -829,11 +829,11 @@ func (c *DefaultCtx) Links(link ...string) { bb := bytebufferpool.Get() for i := range link { if i%2 == 0 { - _ = bb.WriteByte('<') //nolint:errcheck // This will never fail - _, _ = bb.WriteString(link[i]) //nolint:errcheck // This will never fail - _ = bb.WriteByte('>') //nolint:errcheck // This will never fail + bb.WriteByte('<') + bb.WriteString(link[i]) + bb.WriteByte('>') } else { - _, _ = bb.WriteString(`; rel="` + link[i] + `",`) //nolint:errcheck // This will never fail + bb.WriteString(`; rel="` + link[i] + `",`) } } c.setCanonical(HeaderLink, strings.TrimRight(c.app.getString(bb.Bytes()), ",")) @@ -1609,26 +1609,26 @@ func (c *DefaultCtx) String() string { buf := bytebufferpool.Get() // Start with the ID, converting it to a hex string without fmt.Sprintf - buf.WriteByte('#') //nolint:errcheck // It is fine to ignore the error + buf.WriteByte('#') // Convert ID to hexadecimal id := strconv.FormatUint(c.fasthttp.ID(), 16) // Pad with leading zeros to ensure 16 characters for i := 0; i < (16 - len(id)); i++ { - buf.WriteByte('0') //nolint:errcheck // It is fine to ignore the error + buf.WriteByte('0') } - buf.WriteString(id) //nolint:errcheck // It is fine to ignore the error - buf.WriteString(" - ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(id) + buf.WriteString(" - ") // Add local and remote addresses directly - buf.WriteString(c.fasthttp.LocalAddr().String()) //nolint:errcheck // It is fine to ignore the error - buf.WriteString(" <-> ") //nolint:errcheck // It is fine to ignore the error - buf.WriteString(c.fasthttp.RemoteAddr().String()) //nolint:errcheck // It is fine to ignore the error - buf.WriteString(" - ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(c.fasthttp.LocalAddr().String()) + buf.WriteString(" <-> ") + buf.WriteString(c.fasthttp.RemoteAddr().String()) + buf.WriteString(" - ") // Add method and URI - buf.Write(c.fasthttp.Request.Header.Method()) //nolint:errcheck // It is fine to ignore the error - buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error - buf.Write(c.fasthttp.URI().FullURI()) //nolint:errcheck // It is fine to ignore the error + buf.Write(c.fasthttp.Request.Header.Method()) + buf.WriteByte(' ') + buf.Write(c.fasthttp.URI().FullURI()) // Allocate string str := buf.String() diff --git a/ctx_test.go b/ctx_test.go index 93b1a1d4cf..1a7b0bbb98 100644 --- a/ctx_test.go +++ b/ctx_test.go @@ -3763,7 +3763,7 @@ func Test_Ctx_RenderWithBindVars(t *testing.T) { err = c.Render("./.github/testdata/index.tmpl", Map{}) require.NoError(t, err) buf := bytebufferpool.Get() - _, _ = buf.WriteString("overwrite") //nolint:errcheck // This will never fail + buf.WriteString("overwrite") defer bytebufferpool.Put(buf) require.NoError(t, err) @@ -3786,7 +3786,7 @@ func Test_Ctx_RenderWithOverwrittenBind(t *testing.T) { require.NoError(t, err) buf := bytebufferpool.Get() - _, _ = buf.WriteString("overwrite") //nolint:errcheck // This will never fail + buf.WriteString("overwrite") defer bytebufferpool.Put(buf) require.Equal(t, "

Hello from Fiber!

", string(c.Response().Body())) diff --git a/log/default.go b/log/default.go index 96c751a940..a7a60abd23 100644 --- a/log/default.go +++ b/log/default.go @@ -27,8 +27,8 @@ func (l *defaultLogger) privateLog(lv Level, fmtArgs []any) { } level := lv.toString() buf := bytebufferpool.Get() - _, _ = buf.WriteString(level) //nolint:errcheck // It is fine to ignore the error - _, _ = buf.WriteString(fmt.Sprint(fmtArgs...)) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(level) + buf.WriteString(fmt.Sprint(fmtArgs...)) _ = l.stdlog.Output(l.depth, buf.String()) //nolint:errcheck // It is fine to ignore the error buf.Reset() @@ -46,7 +46,7 @@ func (l *defaultLogger) privateLogf(lv Level, format string, fmtArgs []any) { } level := lv.toString() buf := bytebufferpool.Get() - _, _ = buf.WriteString(level) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(level) if len(fmtArgs) > 0 { _, _ = fmt.Fprintf(buf, format, fmtArgs...) @@ -69,11 +69,11 @@ func (l *defaultLogger) privateLogw(lv Level, format string, keysAndValues []any } level := lv.toString() buf := bytebufferpool.Get() - _, _ = buf.WriteString(level) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(level) // Write format privateLog buffer if format != "" { - _, _ = buf.WriteString(format) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(format) } // Write keys and values privateLog buffer if len(keysAndValues) > 0 { @@ -83,11 +83,11 @@ func (l *defaultLogger) privateLogw(lv Level, format string, keysAndValues []any for i := 0; i < len(keysAndValues); i += 2 { if i > 0 || format != "" { - _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + buf.WriteByte(' ') } - _, _ = buf.WriteString(keysAndValues[i].(string)) //nolint:errcheck // It is fine to ignore the error - _ = buf.WriteByte('=') //nolint:errcheck // It is fine to ignore the error - _, _ = buf.WriteString(utils.ToString(keysAndValues[i+1])) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(keysAndValues[i].(string)) //nolint:forcetypeassert // Keys must be strings + buf.WriteByte('=') + buf.WriteString(utils.ToString(keysAndValues[i+1])) } } diff --git a/log/default_test.go b/log/default_test.go index 3b1eb6f06b..9e820b6f4a 100644 --- a/log/default_test.go +++ b/log/default_test.go @@ -81,14 +81,14 @@ func Test_CtxLogger(t *testing.T) { WithContext(ctx).Debugf("received %s order", work) WithContext(ctx).Infof("starting %s", work) WithContext(ctx).Warnf("%s may fail", work) - WithContext(ctx).Errorf("%s failed", work) + WithContext(ctx).Errorf("%s failed %d", work, 50) WithContext(ctx).Panicf("%s panic", work) require.Equal(t, "[Trace] trace work\n"+ "[Debug] received work order\n"+ "[Info] starting work\n"+ "[Warn] work may fail\n"+ - "[Error] work failed\n"+ + "[Error] work failed 50\n"+ "[Panic] work panic\n", string(w.b)) } diff --git a/middleware/etag/etag.go b/middleware/etag/etag.go index 881e18c5e7..2a2c0ae161 100644 --- a/middleware/etag/etag.go +++ b/middleware/etag/etag.go @@ -53,14 +53,14 @@ func New(config ...Config) fiber.Handler { // Enable weak tag if cfg.Weak { - _, _ = bb.Write(weakPrefix) //nolint:errcheck // This will never fail + bb.Write(weakPrefix) } - _ = bb.WriteByte('"') //nolint:errcheck // This will never fail + bb.WriteByte('"') bb.B = appendUint(bb.Bytes(), uint32(len(body))) - _ = bb.WriteByte('-') //nolint:errcheck // This will never fail + bb.WriteByte('-') bb.B = appendUint(bb.Bytes(), crc32.Checksum(body, crc32q)) - _ = bb.WriteByte('"') //nolint:errcheck // This will never fail + bb.WriteByte('"') etag := bb.Bytes() diff --git a/middleware/logger/default_logger.go b/middleware/logger/default_logger.go index 49a9202d93..22da35bf5d 100644 --- a/middleware/logger/default_logger.go +++ b/middleware/logger/default_logger.go @@ -33,9 +33,9 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { if data.ChainErr != nil { formatErr = colors.Red + " | " + data.ChainErr.Error() + colors.Reset } - _, _ = buf.WriteString( //nolint:errcheck // This will never fail + buf.WriteString( fmt.Sprintf("%s |%s %3d %s| %13v | %15s |%s %-7s %s| %-"+data.ErrPaddingStr+"s %s\n", - data.Timestamp.Load().(string), + data.Timestamp.Load().(string), //nolint:forcetypeassert // Timestamp is always a string statusColor(c.Response().StatusCode(), colors), c.Response().StatusCode(), colors.Reset, data.Stop.Sub(data.Start), c.IP(), @@ -53,45 +53,45 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { fixedWidth := func(s string, width int, rightAlign bool) { if rightAlign { for i := len(s); i < width; i++ { - _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + buf.WriteByte(' ') } - _, _ = buf.WriteString(s) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(s) } else { - _, _ = buf.WriteString(s) //nolint:errcheck // It is fine to ignore the error + buf.WriteString(s) for i := len(s); i < width; i++ { - _ = buf.WriteByte(' ') //nolint:errcheck // It is fine to ignore the error + buf.WriteByte(' ') } } } // Timestamp - _, _ = buf.WriteString(data.Timestamp.Load().(string)) //nolint:errcheck // It is fine to ignore the error - _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(data.Timestamp.Load().(string)) //nolint:forcetypeassert // Timestamp is always a string + buf.WriteString(" | ") // Status Code with 3 fixed width, right aligned fixedWidth(strconv.Itoa(c.Response().StatusCode()), 3, true) - _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" | ") // Duration with 13 fixed width, right aligned fixedWidth(data.Stop.Sub(data.Start).String(), 13, true) - _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" | ") // Client IP with 15 fixed width, right aligned fixedWidth(c.IP(), 15, true) - _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" | ") // HTTP Method with 7 fixed width, left aligned fixedWidth(c.Method(), 7, false) - _, _ = buf.WriteString(" | ") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" | ") // Path with dynamic padding for error message, left aligned errPadding, _ := strconv.Atoi(data.ErrPaddingStr) //nolint:errcheck // It is fine to ignore the error fixedWidth(c.Path(), errPadding, false) // Error message - _, _ = buf.WriteString(" ") //nolint:errcheck // It is fine to ignore the error - _, _ = buf.WriteString(formatErr) //nolint:errcheck // It is fine to ignore the error - _, _ = buf.WriteString("\n") //nolint:errcheck // It is fine to ignore the error + buf.WriteString(" ") + buf.WriteString(formatErr) + buf.WriteString("\n") } // Write buffer to output @@ -112,7 +112,7 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { // Loop over template parts execute dynamic parts and add fixed parts to the buffer for i, logFunc := range data.LogFuncChain { if logFunc == nil { - _, _ = buf.Write(data.TemplateChain[i]) //nolint:errcheck // This will never fail + buf.Write(data.TemplateChain[i]) } else if data.TemplateChain[i] == nil { _, err = logFunc(buf, c, data, "") } else { @@ -125,7 +125,7 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { // Also write errors to the buffer if err != nil { - _, _ = buf.WriteString(err.Error()) //nolint:errcheck // This will never fail + buf.WriteString(err.Error()) } mu.Lock() diff --git a/middleware/session/store.go b/middleware/session/store.go index ef4ecb4469..b176e5f596 100644 --- a/middleware/session/store.go +++ b/middleware/session/store.go @@ -75,7 +75,7 @@ func (s *Store) Get(c fiber.Ctx) (*Session, error) { if raw != nil && err == nil { mux.Lock() defer mux.Unlock() - _, _ = sess.byteBuffer.Write(raw) //nolint:errcheck // This will never fail + sess.byteBuffer.Write(raw) encCache := gob.NewDecoder(sess.byteBuffer) err := encCache.Decode(&sess.data.Data) if err != nil { diff --git a/redirect.go b/redirect.go index bfa8ab7756..74b6ad304c 100644 --- a/redirect.go +++ b/redirect.go @@ -207,10 +207,10 @@ func (r *Redirect) Route(name string, config ...RedirectConfig) error { // flash messages for i, message := range r.messages { - _, _ = messageText.WriteString(message) //nolint:errcheck // Always return nil + messageText.WriteString(message) // when there are more messages or oldInput -> add a comma if len(r.messages)-1 != i || (len(r.messages)-1 == i && len(r.oldInput) > 0) { - _, _ = messageText.WriteString(CookieDataSeparator) //nolint:errcheck // Always return nil + messageText.WriteString(CookieDataSeparator) } } r.messages = r.messages[:0] @@ -218,9 +218,9 @@ func (r *Redirect) Route(name string, config ...RedirectConfig) error { // old input data i := 1 for k, v := range r.oldInput { - _, _ = messageText.WriteString(OldInputDataPrefix + k + CookieDataAssigner + v) //nolint:errcheck // Always return nil + messageText.WriteString(OldInputDataPrefix + k + CookieDataAssigner + v) if len(r.oldInput) != i { - _, _ = messageText.WriteString(CookieDataSeparator) //nolint:errcheck // Always return nil + messageText.WriteString(CookieDataSeparator) } i++ } @@ -239,10 +239,10 @@ func (r *Redirect) Route(name string, config ...RedirectConfig) error { i := 1 for k, v := range cfg.Queries { - _, _ = queryText.WriteString(k + "=" + v) //nolint:errcheck // Always return nil + queryText.WriteString(k + "=" + v) if i != len(cfg.Queries) { - _, _ = queryText.WriteString("&") //nolint:errcheck // Always return nil + queryText.WriteString("&") } i++ } diff --git a/redirect_test.go b/redirect_test.go index f3b1e7caa2..33241e3e5e 100644 --- a/redirect_test.go +++ b/redirect_test.go @@ -341,14 +341,17 @@ func Benchmark_Redirect_Route(b *testing.B) { b.ReportAllocs() b.ResetTimer() + var err error + for n := 0; n < b.N; n++ { - c.Redirect().Route("user", RedirectConfig{ //nolint:errcheck,revive // we don't need to handle error here + err = c.Redirect().Route("user", RedirectConfig{ Params: Map{ "name": "fiber", }, }) } + require.NoError(b, err) require.Equal(b, 302, c.Response().StatusCode()) require.Equal(b, "/user/fiber", string(c.Response().Header.Peek(HeaderLocation))) } @@ -365,8 +368,10 @@ func Benchmark_Redirect_Route_WithQueries(b *testing.B) { b.ReportAllocs() b.ResetTimer() + var err error + for n := 0; n < b.N; n++ { - c.Redirect().Route("user", RedirectConfig{ //nolint:errcheck,revive // we don't need to handle error here + err = c.Redirect().Route("user", RedirectConfig{ Params: Map{ "name": "fiber", }, @@ -374,6 +379,7 @@ func Benchmark_Redirect_Route_WithQueries(b *testing.B) { }) } + require.NoError(b, err) require.Equal(b, 302, c.Response().StatusCode()) // analysis of query parameters with url parsing, since a map pass is always randomly ordered location, err := url.Parse(string(c.Response().Header.Peek(HeaderLocation))) @@ -394,10 +400,13 @@ func Benchmark_Redirect_Route_WithFlashMessages(b *testing.B) { b.ReportAllocs() b.ResetTimer() + var err error + for n := 0; n < b.N; n++ { - c.Redirect().With("success", "1").With("message", "test").Route("user") //nolint:errcheck,revive // we don't need to handle error here + err = c.Redirect().With("success", "1").With("message", "test").Route("user") } + require.NoError(b, err) require.Equal(b, 302, c.Response().StatusCode()) require.Equal(b, "/user", string(c.Response().Header.Peek(HeaderLocation))) From 70067a17547c66c7e7674455b297fb0215cb731c Mon Sep 17 00:00:00 2001 From: nickajacks1 <128185314+nickajacks1@users.noreply.github.com> Date: Fri, 9 Feb 2024 16:32:37 -0800 Subject: [PATCH 5/9] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor:=20Remove=20m?= =?UTF-8?q?utex=20lock=20in=20logger=20middleware=20(#2840)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While not all implementations of io.Write will be goroutine safe, the vast majority of users of the logger middleware are likely to use os.File, which does implement safe concurrent writes. If users require locking, they can implement this on an as-needed basis. The risk of having global locking is that a slow write can hold up the entire server. --- docs/api/middleware/logger.md | 4 ++++ middleware/logger/default_logger.go | 5 ----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/api/middleware/logger.md b/docs/api/middleware/logger.md index 8ec69d324c..9208eb4677 100644 --- a/docs/api/middleware/logger.md +++ b/docs/api/middleware/logger.md @@ -88,6 +88,10 @@ app.Use(logger.New(logger.Config{ })) ``` +:::tip +Writing to os.File is goroutine-safe, but if you are using a custom Output that is not goroutine-safe, make sure to implement locking to properly serialize writes. +::: + ## Config ### Config diff --git a/middleware/logger/default_logger.go b/middleware/logger/default_logger.go index 22da35bf5d..0070b72031 100644 --- a/middleware/logger/default_logger.go +++ b/middleware/logger/default_logger.go @@ -5,7 +5,6 @@ import ( "io" "os" "strconv" - "sync" "github.com/gofiber/fiber/v3" "github.com/gofiber/utils/v2" @@ -15,8 +14,6 @@ import ( "github.com/valyala/fasthttp" ) -var mu sync.Mutex - // default logger for fiber func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { // Alias colors @@ -128,9 +125,7 @@ func defaultLoggerInstance(c fiber.Ctx, data *Data, cfg Config) error { buf.WriteString(err.Error()) } - mu.Lock() writeLog(cfg.Output, buf.Bytes()) - mu.Unlock() if cfg.Done != nil { cfg.Done(c, buf.Bytes()) From 97da409533060d3a01ee20409935542010b92b86 Mon Sep 17 00:00:00 2001 From: nickajacks1 <128185314+nickajacks1@users.noreply.github.com> Date: Sat, 10 Feb 2024 10:50:29 -0800 Subject: [PATCH 6/9] =?UTF-8?q?=F0=9F=8E=A8=20Style!:=20Update=20CSRF=20an?= =?UTF-8?q?d=20Limiter=20to=20remove=20repetitive=20names=20(#2846)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit chore!: Update CSRF and Limiter to remove repetitive names The `exported` rule of revive warns to not repeat the package name in method names. For example, prefer `csrf.FromCookie` over `csrf.CsrfFromCookie`. This is a breaking change for v3. It appears that these issues will not be caught by the linter until the `exported` rule is reenabled. This requires comments on all exported symbols, which is a much broader effort. --- .golangci.yml | 2 -- docs/api/middleware/csrf.md | 6 +++--- middleware/csrf/config.go | 12 ++++++------ middleware/csrf/csrf.go | 25 +++++++++++++------------ middleware/csrf/extractors.go | 20 ++++++++++---------- middleware/limiter/config.go | 2 +- middleware/limiter/limiter.go | 2 +- 7 files changed, 34 insertions(+), 35 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index c19f219e77..3ef22c9e84 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -109,8 +109,6 @@ linters-settings: disabled: true - name: exported disabled: true - arguments: - - disableStutteringCheck # TODO https://github.com/gofiber/fiber/issues/2816 - name: file-header disabled: true - name: function-result-limit diff --git a/docs/api/middleware/csrf.md b/docs/api/middleware/csrf.md index cb051577e4..54c2d54897 100644 --- a/docs/api/middleware/csrf.md +++ b/docs/api/middleware/csrf.md @@ -31,7 +31,7 @@ By default, the middleware generates and stores tokens using the `fiber.Storage` When the authorization status changes, the previously issued token MUST be deleted, and a new one generated. See [Token Lifecycle](#token-lifecycle) [Deleting Tokens](#deleting-tokens) for more information. :::caution -When using this pattern, it's important to set the `CookieSameSite` option to `Lax` or `Strict` and ensure that the Extractor is not `CsrfFromCookie`, and KeyLookup is not `cookie:`. +When using this pattern, it's important to set the `CookieSameSite` option to `Lax` or `Strict` and ensure that the Extractor is not `FromCookie`, and KeyLookup is not `cookie:`. ::: :::note @@ -206,7 +206,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), SessionKey: "csrfToken", } ``` @@ -226,7 +226,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), Session: session.Store, SessionKey: "csrfToken", } diff --git a/middleware/csrf/config.go b/middleware/csrf/config.go index cb26cb23db..3902555dbc 100644 --- a/middleware/csrf/config.go +++ b/middleware/csrf/config.go @@ -117,7 +117,7 @@ var ConfigDefault = Config{ Expiration: 1 * time.Hour, KeyGenerator: utils.UUIDv4, ErrorHandler: defaultErrorHandler, - Extractor: CsrfFromHeader(HeaderName), + Extractor: FromHeader(HeaderName), SessionKey: "csrfToken", } @@ -169,15 +169,15 @@ func configDefault(config ...Config) Config { if cfg.Extractor == nil { // By default we extract from a header - cfg.Extractor = CsrfFromHeader(textproto.CanonicalMIMEHeaderKey(selectors[1])) + cfg.Extractor = FromHeader(textproto.CanonicalMIMEHeaderKey(selectors[1])) switch selectors[0] { case "form": - cfg.Extractor = CsrfFromForm(selectors[1]) + cfg.Extractor = FromForm(selectors[1]) case "query": - cfg.Extractor = CsrfFromQuery(selectors[1]) + cfg.Extractor = FromQuery(selectors[1]) case "param": - cfg.Extractor = CsrfFromParam(selectors[1]) + cfg.Extractor = FromParam(selectors[1]) case "cookie": if cfg.Session == nil { log.Warn("[CSRF] Cookie extractor is not recommended without a session store") @@ -185,7 +185,7 @@ func configDefault(config ...Config) Config { if cfg.CookieSameSite == "None" || cfg.CookieSameSite != "Lax" && cfg.CookieSameSite != "Strict" { log.Warn("[CSRF] Cookie extractor is only recommended for use with SameSite=Lax or SameSite=Strict") } - cfg.Extractor = CsrfFromCookie(selectors[1]) + cfg.Extractor = FromCookie(selectors[1]) cfg.CookieName = selectors[1] // Cookie name is the same as the key } } diff --git a/middleware/csrf/csrf.go b/middleware/csrf/csrf.go index 9801afd3dc..644d39019f 100644 --- a/middleware/csrf/csrf.go +++ b/middleware/csrf/csrf.go @@ -17,7 +17,8 @@ var ( dummyValue = []byte{'+'} ) -type CSRFHandler struct { +// Handler handles +type Handler struct { config *Config sessionManager *sessionManager storageManager *storageManager @@ -58,7 +59,7 @@ func New(config ...Config) fiber.Handler { } // Store the CSRF handler in the context - c.Locals(handlerKey, &CSRFHandler{ + c.Locals(handlerKey, &Handler{ config: &cfg, sessionManager: sessionManager, storageManager: storageManager, @@ -98,10 +99,10 @@ func New(config ...Config) fiber.Handler { return cfg.ErrorHandler(c, ErrTokenNotFound) } - // If not using CsrfFromCookie extractor, check that the token matches the cookie + // If not using FromCookie extractor, check that the token matches the cookie // This is to prevent CSRF attacks by using a Double Submit Cookie method // Useful when we do not have access to the users Session - if !isCsrfFromCookie(cfg.Extractor) && !compareStrings(extractedToken, c.Cookies(cfg.CookieName)) { + if !isFromCookie(cfg.Extractor) && !compareStrings(extractedToken, c.Cookies(cfg.CookieName)) { return cfg.ErrorHandler(c, ErrTokenInvalid) } @@ -154,10 +155,10 @@ func TokenFromContext(c fiber.Ctx) string { return token } -// HandlerFromContext returns the CSRFHandler found in the context +// HandlerFromContext returns the Handler found in the context // returns nil if the handler does not exist -func HandlerFromContext(c fiber.Ctx) *CSRFHandler { - handler, ok := c.Locals(handlerKey).(*CSRFHandler) +func HandlerFromContext(c fiber.Ctx) *Handler { + handler, ok := c.Locals(handlerKey).(*Handler) if !ok { return nil } @@ -219,11 +220,11 @@ func setCSRFCookie(c fiber.Ctx, cfg Config, token string, expiry time.Duration) // DeleteToken removes the token found in the context from the storage // and expires the CSRF cookie -func (handler *CSRFHandler) DeleteToken(c fiber.Ctx) error { +func (handler *Handler) DeleteToken(c fiber.Ctx) error { // Get the config from the context config := handler.config if config == nil { - panic("CSRFHandler config not found in context") + panic("CSRF Handler config not found in context") } // Extract token from the client request cookie cookieToken := c.Cookies(config.CookieName) @@ -237,9 +238,9 @@ func (handler *CSRFHandler) DeleteToken(c fiber.Ctx) error { return nil } -// isCsrfFromCookie checks if the extractor is set to ExtractFromCookie -func isCsrfFromCookie(extractor any) bool { - return reflect.ValueOf(extractor).Pointer() == reflect.ValueOf(CsrfFromCookie).Pointer() +// isFromCookie checks if the extractor is set to ExtractFromCookie +func isFromCookie(extractor any) bool { + return reflect.ValueOf(extractor).Pointer() == reflect.ValueOf(FromCookie).Pointer() } // refererMatchesHost checks that the referer header matches the host header diff --git a/middleware/csrf/extractors.go b/middleware/csrf/extractors.go index 1396d2ec02..8e4a119f9e 100644 --- a/middleware/csrf/extractors.go +++ b/middleware/csrf/extractors.go @@ -14,8 +14,8 @@ var ( ErrMissingCookie = errors.New("missing csrf token in cookie") ) -// csrfFromParam returns a function that extracts token from the url param string. -func CsrfFromParam(param string) func(c fiber.Ctx) (string, error) { +// FromParam returns a function that extracts token from the url param string. +func FromParam(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Params(param) if token == "" { @@ -25,8 +25,8 @@ func CsrfFromParam(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromForm returns a function that extracts a token from a multipart-form. -func CsrfFromForm(param string) func(c fiber.Ctx) (string, error) { +// FromForm returns a function that extracts a token from a multipart-form. +func FromForm(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.FormValue(param) if token == "" { @@ -36,8 +36,8 @@ func CsrfFromForm(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromCookie returns a function that extracts token from the cookie header. -func CsrfFromCookie(param string) func(c fiber.Ctx) (string, error) { +// FromCookie returns a function that extracts token from the cookie header. +func FromCookie(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Cookies(param) if token == "" { @@ -47,8 +47,8 @@ func CsrfFromCookie(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromHeader returns a function that extracts token from the request header. -func CsrfFromHeader(param string) func(c fiber.Ctx) (string, error) { +// FromHeader returns a function that extracts token from the request header. +func FromHeader(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := c.Get(param) if token == "" { @@ -58,8 +58,8 @@ func CsrfFromHeader(param string) func(c fiber.Ctx) (string, error) { } } -// csrfFromQuery returns a function that extracts token from the query string. -func CsrfFromQuery(param string) func(c fiber.Ctx) (string, error) { +// FromQuery returns a function that extracts token from the query string. +func FromQuery(param string) func(c fiber.Ctx) (string, error) { return func(c fiber.Ctx) (string, error) { token := fiber.Query[string](c, param) if token == "" { diff --git a/middleware/limiter/config.go b/middleware/limiter/config.go index 291c85d491..3045282eec 100644 --- a/middleware/limiter/config.go +++ b/middleware/limiter/config.go @@ -55,7 +55,7 @@ type Config struct { // LimiterMiddleware is the struct that implements a limiter middleware. // // Default: a new Fixed Window Rate Limiter - LimiterMiddleware LimiterHandler + LimiterMiddleware Handler } // ConfigDefault is the default config diff --git a/middleware/limiter/limiter.go b/middleware/limiter/limiter.go index e3e239f0a9..f6bba52667 100644 --- a/middleware/limiter/limiter.go +++ b/middleware/limiter/limiter.go @@ -11,7 +11,7 @@ const ( xRateLimitReset = "X-RateLimit-Reset" ) -type LimiterHandler interface { +type Handler interface { New(config Config) fiber.Handler } From 389e63d2c207038639062ff9a7bd73592cdc0696 Mon Sep 17 00:00:00 2001 From: RW Date: Sat, 10 Feb 2024 19:51:20 +0100 Subject: [PATCH 7/9] Update csrf.md fix readme example --- docs/api/middleware/csrf.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/api/middleware/csrf.md b/docs/api/middleware/csrf.md index 54c2d54897..e6c1425e91 100644 --- a/docs/api/middleware/csrf.md +++ b/docs/api/middleware/csrf.md @@ -146,8 +146,6 @@ KeyLookup will be ignored if Extractor is explicitly set. Getting the CSRF token in a handler: -```go - ```go func handler(c fiber.Ctx) error { handler := csrf.HandlerFromContext(c) From 573afb953d9b8288c4f5f7e32cb2a52c9d607aa2 Mon Sep 17 00:00:00 2001 From: Martin Hotmann Date: Sat, 10 Feb 2024 21:55:04 +0100 Subject: [PATCH 8/9] Update startup message formatting (#2847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update listen.go (unform INFO prints at startip + cosmetics) BEFORE: ``` _______ __ / ____(_) /_ ___ _____ / /_ / / __ \/ _ \/ ___/ / __/ / / /_/ / __/ / /_/ /_/_.___/\___/_/ v3.0.0-beta.1 -------------------------------------------------- INFO Server started on http://127.0.0.1:8003 (bound on host 0.0.0.0 and port 8003) INFO Application name: TEST APP INFO Total handlers count: 5 INFO Prefork: Disabled INFO PID: 2342 INFO Total process count: 1 20:49:50 | 200 | 593.769µs | 123.123.123.123 | GET | / ``` AFTER: ``` _______ __ / ____(_) /_ ___ _____ / /_ / / __ \/ _ \/ ___/ / __/ / / /_/ / __/ / /_/ /_/_.___/\___/_/ v3.0.0-beta.1 -------------------------------------------------- INFO Server started on: http://127.0.0.1:8003 (bound on host 0.0.0.0 and port 8003) INFO Application name: TEST APP INFO Total handlers count: 5 INFO Prefork: Disabled INFO PID: 2342 INFO Total process count: 1 20:49:50 | 200 | 593.769µs | 123.123.123.123 | GET | / ``` * fix spacer * fix indentation for fiber version * fix linting * fix linting #2 * fix listen_test.go to match newly expected output. * fix test again --------- Co-authored-by: root Co-authored-by: Martin --- listen.go | 23 +++++++++++++---------- listen_test.go | 4 ++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/listen.go b/listen.go index f27d721736..e293a1ef7c 100644 --- a/listen.go +++ b/listen.go @@ -30,7 +30,7 @@ var figletFiberText = ` / ____(_) /_ ___ _____ / /_ / / __ \/ _ \/ ___/ / __/ / / /_/ / __/ / -/_/ /_/_.___/\___/_/ %s` +/_/ /_/_.___/\___/_/ %s` const ( globalIpv4Addr = "0.0.0.0" @@ -354,27 +354,27 @@ func (app *App) startupMessage(addr string, isTLS bool, pids string, cfg ListenC if host == "0.0.0.0" { _, _ = fmt.Fprintf(out, - "%sINFO%s Server started on %s%s://127.0.0.1:%s%s (bound on host 0.0.0.0 and port %s)\n", + "%sINFO%s Server started on: \t%s%s://127.0.0.1:%s%s (bound on host 0.0.0.0 and port %s)\n", colors.Green, colors.Reset, colors.Blue, scheme, port, colors.Reset, port) } else { _, _ = fmt.Fprintf(out, - "%sINFO%s Server started on %s%s%s\n", + "%sINFO%s Server started on: \t%s%s%s\n", colors.Green, colors.Reset, colors.Blue, fmt.Sprintf("%s://%s:%s", scheme, host, port), colors.Reset) } if app.config.AppName != "" { - _, _ = fmt.Fprintf(out, "%sINFO%s Application name: %s%s%s\n", colors.Green, colors.Reset, colors.Blue, app.config.AppName, colors.Reset) + _, _ = fmt.Fprintf(out, "%sINFO%s Application name: \t\t%s%s%s\n", colors.Green, colors.Reset, colors.Blue, app.config.AppName, colors.Reset) } _, _ = fmt.Fprintf(out, - "%sINFO%s Total handlers count: %s%s%s\n", + "%sINFO%s Total handlers count: \t%s%s%s\n", colors.Green, colors.Reset, colors.Blue, strconv.Itoa(int(app.handlersCount)), colors.Reset) if isPrefork == "Enabled" { - _, _ = fmt.Fprintf(out, "%sINFO%s Prefork: %s%s%s\n", colors.Green, colors.Reset, colors.Blue, isPrefork, colors.Reset) + _, _ = fmt.Fprintf(out, "%sINFO%s Prefork: \t\t\t%s%s%s\n", colors.Green, colors.Reset, colors.Blue, isPrefork, colors.Reset) } else { - _, _ = fmt.Fprintf(out, "%sINFO%s Prefork: %s%s%s\n", colors.Green, colors.Reset, colors.Red, isPrefork, colors.Reset) + _, _ = fmt.Fprintf(out, "%sINFO%s Prefork: \t\t\t%s%s%s\n", colors.Green, colors.Reset, colors.Red, isPrefork, colors.Reset) } - _, _ = fmt.Fprintf(out, "%sINFO%s PID: %s%v%s\n", colors.Green, colors.Reset, colors.Blue, os.Getpid(), colors.Reset) - _, _ = fmt.Fprintf(out, "%sINFO%s Total process count: %s%s%s\n", colors.Green, colors.Reset, colors.Blue, procs, colors.Reset) + _, _ = fmt.Fprintf(out, "%sINFO%s PID: \t\t\t%s%v%s\n", colors.Green, colors.Reset, colors.Blue, os.Getpid(), colors.Reset) + _, _ = fmt.Fprintf(out, "%sINFO%s Total process count: \t%s%s%s\n", colors.Green, colors.Reset, colors.Blue, procs, colors.Reset) if cfg.EnablePrefork { // Turn the `pids` variable (in the form ",a,b,c,d,e,f,etc") into a slice of PIDs @@ -385,7 +385,7 @@ func (app *App) startupMessage(addr string, isTLS bool, pids string, cfg ListenC } } - _, _ = fmt.Fprintf(out, "%sINFO%s Child PIDs: %s", colors.Green, colors.Reset, colors.Blue) + _, _ = fmt.Fprintf(out, "%sINFO%s Child PIDs: \t\t%s", colors.Green, colors.Reset, colors.Blue) totalPids := len(pidSlice) rowTotalPidCount := 10 for i := 0; i < totalPids; i += rowTotalPidCount { @@ -403,6 +403,9 @@ func (app *App) startupMessage(addr string, isTLS bool, pids string, cfg ListenC _, _ = fmt.Fprintf(out, "\n%s", colors.Reset) } } + + // add new Line as spacer + _, _ = fmt.Fprintf(out, "\n%s", colors.Reset) } // printRoutesMessage print all routes with method, path, name and handlers diff --git a/listen_test.go b/listen_test.go index 9134e357a2..ffd5379402 100644 --- a/listen_test.go +++ b/listen_test.go @@ -355,7 +355,7 @@ func Test_Listen_Master_Process_Show_Startup_Message(t *testing.T) { require.Contains(t, startupMessage, "(bound on host 0.0.0.0 and port 3000)") require.Contains(t, startupMessage, "Child PIDs") require.Contains(t, startupMessage, "11111, 22222, 33333, 44444, 55555, 60000") - require.Contains(t, startupMessage, fmt.Sprintf("Prefork: %sEnabled%s", colors.Blue, colors.Reset)) + require.Contains(t, startupMessage, fmt.Sprintf("Prefork: \t\t\t%sEnabled%s", colors.Blue, colors.Reset)) } // go test -run Test_Listen_Master_Process_Show_Startup_MessageWithAppName @@ -402,7 +402,7 @@ func Test_Listen_Master_Process_Show_Startup_MessageWithDisabledPreforkAndCustom require.Contains(t, startupMessage, fmt.Sprintf("%sINFO%s", colors.Green, colors.Reset)) require.Contains(t, startupMessage, fmt.Sprintf("%s%s%s", colors.Blue, appName, colors.Reset)) require.Contains(t, startupMessage, fmt.Sprintf("%s%s%s", colors.Blue, "https://server.com:8081", colors.Reset)) - require.Contains(t, startupMessage, fmt.Sprintf("Prefork: %sDisabled%s", colors.Red, colors.Reset)) + require.Contains(t, startupMessage, fmt.Sprintf("Prefork: \t\t\t%sDisabled%s", colors.Red, colors.Reset)) } // go test -run Test_Listen_Print_Route From 92dd8d691702ba4628a4b5f326c7fb31d93714d8 Mon Sep 17 00:00:00 2001 From: nickajacks1 <128185314+nickajacks1@users.noreply.github.com> Date: Sat, 10 Feb 2024 13:36:49 -0800 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=8E=A8=20Style:=20Add=20inamedparam?= =?UTF-8?q?=20linter=20(#2848)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit inamedparam enforces that parameters in interface definitions be named. This is important for clarity so that users and implementers can easily understand the purpose of each parameter. --- .golangci.yml | 2 +- bind.go | 4 ++-- ctx.go | 2 +- log/log.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3ef22c9e84..50b6c3d54f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -191,7 +191,7 @@ linters: # - gosmopolitan # TODO https://github.com/gofiber/fiber/issues/2816 - govet - grouper - # - inamedparam # TODO https://github.com/gofiber/fiber/issues/2816 + - inamedparam - loggercheck # - mirror # TODO https://github.com/gofiber/fiber/issues/2816 - misspell diff --git a/bind.go b/bind.go index ab3594944f..0c9a63c4a8 100644 --- a/bind.go +++ b/bind.go @@ -9,13 +9,13 @@ import ( type CustomBinder interface { Name() string MIMETypes() []string - Parse(Ctx, any) error + Parse(c Ctx, out any) error } // An interface to register custom struct validator for binding. type StructValidator interface { Engine() any - ValidateStruct(any) error + ValidateStruct(out any) error } // Bind struct diff --git a/ctx.go b/ctx.go index d58ef263b7..5421674276 100644 --- a/ctx.go +++ b/ctx.go @@ -105,7 +105,7 @@ type Cookie struct { // Views is the interface that wraps the Render function. type Views interface { Load() error - Render(io.Writer, string, any, ...string) error + Render(out io.Writer, name string, binding any, layout ...string) error } // ResFmt associates a Content Type to a fiber.Handler for c.Format diff --git a/log/log.go b/log/log.go index 11698135cf..9d9cd8b0d2 100644 --- a/log/log.go +++ b/log/log.go @@ -54,8 +54,8 @@ type CommonLogger interface { // ControlLogger provides methods to config a logger. type ControlLogger interface { - SetLevel(Level) - SetOutput(io.Writer) + SetLevel(level Level) + SetOutput(w io.Writer) } // AllLogger is the combination of Logger, FormatLogger, CtxLogger and ControlLogger.