Add warning for deprecated newline separator (#2031)

* Add warning for deprecated newline separator

* Fix feedback, no context by default, use slog
This commit is contained in:
Erik Dubbelboer
2025-07-10 12:47:32 +08:00
committed by GitHub
parent eb1f908d97
commit a3c9dab757
2 changed files with 101 additions and 0 deletions
+35
View File
@@ -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
+66
View File
@@ -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()