From a3c9dab7573946aa0afcee62d94fbfb58e3c4c2c Mon Sep 17 00:00:00 2001 From: Erik Dubbelboer Date: Thu, 10 Jul 2025 12:47:32 +0800 Subject: [PATCH] Add warning for deprecated newline separator (#2031) * Add warning for deprecated newline separator * Fix feedback, no context by default, use slog --- header.go | 35 ++++++++++++++++++++++++++ header_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/header.go b/header.go index f420a6c..0f262bc 100644 --- a/header.go +++ b/header.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "iter" + "log/slog" "sync" "sync/atomic" "time" @@ -3100,8 +3101,21 @@ type headerScanner struct { nextNewLine int initialized bool + + // This is only used to print the deprecated newline separator warning. + // TODO: Remove this again once the newline separator is removed. + warned bool } +// DeprecatedNewlineIncludeContext is used to control whether the context of the +// header is included in the warning message about the deprecated newline +// separator. +// Warning: this can potentially leak sensitive information such as auth headers. +var DeprecatedNewlineIncludeContext atomic.Bool + +// TODO: Remove this again once the newline separator is removed. +var warnedAboutDeprecatedNewlineSeparatorLimiter atomic.Int64 + func (s *headerScanner) next() bool { if !s.initialized { s.nextColon = -1 @@ -3139,6 +3153,27 @@ func (s *headerScanner) next() bool { s.err = errInvalidName return false } + + // If the character before '\n' isn't '\r', print a warning. + if !s.warned && x > 1 && s.b[x-1] != rChar { + // Only warn once per second. + now := time.Now().Unix() + if warnedAboutDeprecatedNewlineSeparatorLimiter.Load() < now { + if warnedAboutDeprecatedNewlineSeparatorLimiter.Swap(now) < now { + if DeprecatedNewlineIncludeContext.Load() { + // Include 20 characters after the '\n'. + xx := x + 20 + if len(s.b) < xx { + xx = len(s.b) + } + slog.Error("Deprecated newline only separator found in header", "context", fmt.Sprintf("%q", s.b[:xx])) + } else { + slog.Error("Deprecated newline only separator found in header") + } + s.warned = true + } + } + } } if n < 0 { s.err = errNeedMore diff --git a/header_test.go b/header_test.go index 08eb2e2..b554432 100644 --- a/header_test.go +++ b/header_test.go @@ -3,10 +3,12 @@ package fasthttp import ( "bufio" "bytes" + "context" "encoding/base64" "errors" "fmt" "io" + "log/slog" "net/http" "reflect" "strconv" @@ -14,6 +16,70 @@ import ( "testing" ) +type slogTestHandler struct { + out string +} + +func (s *slogTestHandler) Enabled(_ context.Context, level slog.Level) bool { + return true +} + +func (s *slogTestHandler) Handle(ctx context.Context, record slog.Record) error { //nolint:gocritic + s.out += record.Message + for r := range record.Attrs { + s.out += " " + r.Key + ":" + r.Value.String() + } + return nil +} + +func (s *slogTestHandler) WithAttrs(attrs []slog.Attr) slog.Handler { + for _, attr := range attrs { + s.out += attr.String() + } + return &slogTestHandler{out: s.out} +} + +func (s *slogTestHandler) WithGroup(name string) slog.Handler { + return &slogTestHandler{out: s.out} +} + +func TestNewlineBackwardsCompatibleWarning(t *testing.T) { + h := &ResponseHeader{} + + l := slog.Default() + ll := &slogTestHandler{} + slog.SetDefault(slog.New(ll)) + defer slog.SetDefault(l) + + DeprecatedNewlineIncludeContext.Store(true) + warnedAboutDeprecatedNewlineSeparatorLimiter.Store(0) + + if err := h.Read(bufio.NewReader(bytes.NewBufferString("HTTP/1.1 200 OK\r\nContent-Type: foo/bar\nContent-Length: 12345\r\n\r\nsss"))); err != nil { + t.Fatal(err) + } + e := h.Peek(HeaderContentType) + if string(e) != "foo/bar" { + t.Fatalf("Unexpected Content-Type %q. Expected %q", e, "foo/bar") + } + expected := "Deprecated newline only separator found in header context:\"Content-Type: foo/bar\\nContent-Length: 123\"" + if ll.out != expected { + t.Errorf("Expected %q, got %q", expected, ll.out) + } + + ll.out = "" + + DeprecatedNewlineIncludeContext.Store(false) + warnedAboutDeprecatedNewlineSeparatorLimiter.Store(0) + + if err := h.Read(bufio.NewReader(bytes.NewBufferString("HTTP/1.1 200 OK\r\nContent-Type: foo/bar\nContent-Length: 12345\r\n\r\nsss"))); err != nil { + t.Fatal(err) + } + expected = "Deprecated newline only separator found in header" + if ll.out != expected { + t.Errorf("Expected %q, got %q", expected, ll.out) + } +} + func TestResponseHeaderAddContentType(t *testing.T) { t.Parallel()