fix(review): 完成系统性 Review 修复方案 - Task B-01 HTTP Server 超时配置
本次提交包含: - B-01: HTTP Server 添加超时配置 (ReadTimeout/WriteTimeout/IdleTimeout/MaxHeaderBytes) - 添加结构化日志包 internal/log/ (B-02 部分完成) - 添加 Review 报告文档 - 添加系统性修复方案文档 - 添加最佳实践审核报告文档 - 更新任务清单和执行板 测试验证: - TestServerHasTimeoutConfiguration 通过 关联文档: - docs/2026-06-01-SYSTEMATIC-REVIEW-REPORT.md - docs/2026-06-01-SYSTEMATIC-REPAIR-PLAN.md - docs/2026-06-01-BEST-PRACTICE-AUDIT-REPORT.md
This commit is contained in:
@@ -21,8 +21,13 @@ func NewServer(listenAddr string, handler http.Handler, listenerFactory Listener
|
||||
}
|
||||
server := &Server{
|
||||
server: &http.Server{
|
||||
Addr: listenAddr,
|
||||
Handler: handler,
|
||||
Addr: listenAddr,
|
||||
Handler: handler,
|
||||
ReadTimeout: 30 * time.Second,
|
||||
ReadHeaderTimeout: 10 * time.Second,
|
||||
WriteTimeout: 30 * time.Second,
|
||||
IdleTimeout: 120 * time.Second,
|
||||
MaxHeaderBytes: 1 << 20, // 1MB
|
||||
},
|
||||
listen: net.Listen,
|
||||
}
|
||||
|
||||
@@ -741,6 +741,32 @@ func TestServerAddrReturnsConfiguredAddress(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestServerHasTimeoutConfiguration(t *testing.T) {
|
||||
server := NewServer("127.0.0.1:0", nil, nil)
|
||||
|
||||
s := server.server
|
||||
|
||||
if s.ReadTimeout != 30*time.Second {
|
||||
t.Errorf("ReadTimeout = %v, want 30s", s.ReadTimeout)
|
||||
}
|
||||
|
||||
if s.ReadHeaderTimeout != 10*time.Second {
|
||||
t.Errorf("ReadHeaderTimeout = %v, want 10s", s.ReadHeaderTimeout)
|
||||
}
|
||||
|
||||
if s.WriteTimeout != 30*time.Second {
|
||||
t.Errorf("WriteTimeout = %v, want 30s", s.WriteTimeout)
|
||||
}
|
||||
|
||||
if s.IdleTimeout != 120*time.Second {
|
||||
t.Errorf("IdleTimeout = %v, want 120s", s.IdleTimeout)
|
||||
}
|
||||
|
||||
if s.MaxHeaderBytes != 1<<20 {
|
||||
t.Errorf("MaxHeaderBytes = %d, want %d", s.MaxHeaderBytes, 1<<20)
|
||||
}
|
||||
}
|
||||
|
||||
func TestClassifyError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
137
internal/log/log.go
Normal file
137
internal/log/log.go
Normal file
@@ -0,0 +1,137 @@
|
||||
// Package log provides structured logging using slog.
|
||||
// It supports JSON output, configurable log levels, and sensitive field sanitization.
|
||||
package log
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"log/slog"
|
||||
)
|
||||
|
||||
var logger *slog.Logger
|
||||
|
||||
// sensitiveFields contains field names that should be sanitized in logs
|
||||
var sensitiveFields = []string{
|
||||
"token",
|
||||
"password",
|
||||
"secret",
|
||||
"key",
|
||||
"credential",
|
||||
"auth",
|
||||
"api_key",
|
||||
"api_secret",
|
||||
"private_key",
|
||||
"access_token",
|
||||
"refresh_token",
|
||||
}
|
||||
|
||||
// Init initializes the logger with JSON handler and INFO level
|
||||
func Init() {
|
||||
InitWithLevel("INFO")
|
||||
}
|
||||
|
||||
// InitWithLevel initializes the logger with specified level
|
||||
func InitWithLevel(level string) {
|
||||
levelVar := parseLevel(level)
|
||||
|
||||
opts := &slog.HandlerOptions{
|
||||
Level: levelVar,
|
||||
ReplaceAttr: sanitizeAttrs,
|
||||
}
|
||||
|
||||
handler := slog.NewJSONHandler(os.Stdout, opts)
|
||||
logger = slog.New(handler)
|
||||
slog.SetDefault(logger)
|
||||
}
|
||||
|
||||
// parseLevel parses string level to slog.Level
|
||||
func parseLevel(level string) slog.Level {
|
||||
switch strings.ToUpper(level) {
|
||||
case "DEBUG":
|
||||
return slog.LevelDebug
|
||||
case "INFO":
|
||||
return slog.LevelInfo
|
||||
case "WARN", "WARNING":
|
||||
return slog.LevelWarn
|
||||
case "ERROR":
|
||||
return slog.LevelError
|
||||
default:
|
||||
return slog.LevelInfo
|
||||
}
|
||||
}
|
||||
|
||||
// sanitizeAttrs sanitizes sensitive fields in log attributes
|
||||
func sanitizeAttrs(groups []string, a slog.Attr) slog.Attr {
|
||||
// Check if attribute key contains sensitive field name
|
||||
keyLower := strings.ToLower(a.Key)
|
||||
for _, field := range sensitiveFields {
|
||||
if strings.Contains(keyLower, field) {
|
||||
return slog.String(a.Key, "[REDACTED]")
|
||||
}
|
||||
}
|
||||
return a
|
||||
}
|
||||
|
||||
// IsSensitive checks if a field name is sensitive
|
||||
func IsSensitive(field string) bool {
|
||||
fieldLower := strings.ToLower(field)
|
||||
for _, f := range sensitiveFields {
|
||||
if strings.Contains(fieldLower, f) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Info logs an info level message
|
||||
func Info(msg string, args ...any) {
|
||||
slog.Info(msg, args...)
|
||||
}
|
||||
|
||||
// Error logs an error level message
|
||||
func Error(msg string, args ...any) {
|
||||
slog.Error(msg, args...)
|
||||
}
|
||||
|
||||
// Debug logs a debug level message
|
||||
func Debug(msg string, args ...any) {
|
||||
slog.Debug(msg, args...)
|
||||
}
|
||||
|
||||
// Warn logs a warning level message
|
||||
func Warn(msg string, args ...any) {
|
||||
slog.Warn(msg, args...)
|
||||
}
|
||||
|
||||
// Logger returns the underlying logger
|
||||
func Logger() *slog.Logger {
|
||||
return logger
|
||||
}
|
||||
|
||||
// WithContext returns a logger with context
|
||||
func WithContext(ctx context.Context) *slog.Logger {
|
||||
// Extract trace_id from context if present
|
||||
if traceID, ok := ctx.Value("trace_id").(string); ok {
|
||||
return logger.With("trace_id", traceID)
|
||||
}
|
||||
return logger
|
||||
}
|
||||
|
||||
// RequestLogger returns a logger with request fields
|
||||
func RequestLogger(method, path, clientIP string) *slog.Logger {
|
||||
return logger.With(
|
||||
"method", method,
|
||||
"path", path,
|
||||
"client_ip", clientIP,
|
||||
"time", time.Now().UTC(),
|
||||
)
|
||||
}
|
||||
|
||||
// Fatal logs an error message and exits with code 1
|
||||
func Fatal(msg string, args ...any) {
|
||||
slog.Error(msg, args...)
|
||||
os.Exit(1)
|
||||
}
|
||||
134
internal/log/log_test.go
Normal file
134
internal/log/log_test.go
Normal file
@@ -0,0 +1,134 @@
|
||||
package log
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"log/slog"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestInit(t *testing.T) {
|
||||
// Save original stdout
|
||||
oldLogger := logger
|
||||
defer func() { logger = oldLogger }()
|
||||
|
||||
Init()
|
||||
|
||||
if logger == nil {
|
||||
t.Error("logger should not be nil after Init")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInitWithLevel(t *testing.T) {
|
||||
// Test different levels
|
||||
levels := []string{"DEBUG", "INFO", "WARN", "ERROR", "unknown"}
|
||||
|
||||
for _, level := range levels {
|
||||
InitWithLevel(level)
|
||||
if logger == nil {
|
||||
t.Errorf("logger should not be nil for level %s", level)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseLevel(t *testing.T) {
|
||||
tests := []struct {
|
||||
input string
|
||||
expected slog.Level
|
||||
}{
|
||||
{"DEBUG", slog.LevelDebug},
|
||||
{"debug", slog.LevelDebug},
|
||||
{"INFO", slog.LevelInfo},
|
||||
{"info", slog.LevelInfo},
|
||||
{"WARN", slog.LevelWarn},
|
||||
{"WARNING", slog.LevelWarn},
|
||||
{"ERROR", slog.LevelError},
|
||||
{"error", slog.LevelError},
|
||||
{"unknown", slog.LevelInfo},
|
||||
{"", slog.LevelInfo},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
result := parseLevel(test.input)
|
||||
if result != test.expected {
|
||||
t.Errorf("parseLevel(%q) = %v, want %v", test.input, result, test.expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsSensitive(t *testing.T) {
|
||||
sensitive := []string{
|
||||
"token",
|
||||
"password",
|
||||
"secret",
|
||||
"api_key",
|
||||
"access_token",
|
||||
"PRIVATE_KEY",
|
||||
}
|
||||
|
||||
for _, field := range sensitive {
|
||||
if !IsSensitive(field) {
|
||||
t.Errorf("IsSensitive(%q) should be true", field)
|
||||
}
|
||||
}
|
||||
|
||||
notSensitive := []string{
|
||||
"name",
|
||||
"email",
|
||||
"user_id",
|
||||
}
|
||||
|
||||
for _, field := range notSensitive {
|
||||
if IsSensitive(field) {
|
||||
t.Errorf("IsSensitive(%q) should be false", field)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeAttrs(t *testing.T) {
|
||||
// Test that sensitive fields are redacted
|
||||
tests := []struct {
|
||||
key string
|
||||
value string
|
||||
expected string
|
||||
}{
|
||||
{"password", "secret123", "[REDACTED]"},
|
||||
{"api_token", "abc123", "[REDACTED]"},
|
||||
{"secret_key", "xyz789", "[REDACTED]"},
|
||||
{"name", "test", "test"},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
attr := slog.String(test.key, test.value)
|
||||
result := sanitizeAttrs(nil, attr)
|
||||
if result.Value.String() != test.expected {
|
||||
t.Errorf("sanitizeAttrs(%q) = %q, want %q", test.key, result.Value.String(), test.expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoggingMethods(t *testing.T) {
|
||||
// Just verify methods don't panic
|
||||
Init()
|
||||
|
||||
Info("test info message", "key", "value")
|
||||
Debug("test debug message", "key", "value")
|
||||
Warn("test warn message", "key", "value")
|
||||
Error("test error message", "key", "value")
|
||||
}
|
||||
|
||||
func TestLogger(t *testing.T) {
|
||||
Init()
|
||||
l := Logger()
|
||||
if l == nil {
|
||||
t.Error("Logger() should not return nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequestLogger(t *testing.T) {
|
||||
Init()
|
||||
l := RequestLogger("GET", "/api/hosts", "127.0.0.1")
|
||||
if l == nil {
|
||||
t.Error("RequestLogger should not return nil")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user