chore(lint): upgrade golangci-lint to v1.47.2

- Fix Slowloris attacks on HTTP servers
- Force set default of 5 minutes for pprof read timeout
- Change `ShutdownTimeout` to time.Duration since it cannot be set to 0
This commit is contained in:
Quentin McGaw
2022-07-28 21:55:10 +00:00
parent 877617cc53
commit a6f00f2fb2
24 changed files with 348 additions and 158 deletions

View File

@@ -2,13 +2,10 @@ package httpserver
import (
"regexp"
"time"
gomock "github.com/golang/mock/gomock"
)
func durationPtr(d time.Duration) *time.Duration { return &d }
var _ Logger = (*testLogger)(nil)
type testLogger struct{}

View File

@@ -11,7 +11,12 @@ import (
// The done channel has an error written to when the HTTP server
// is terminated, and can be nil or not nil.
func (s *Server) Run(ctx context.Context, ready chan<- struct{}, done chan<- struct{}) {
server := http.Server{Addr: s.address, Handler: s.handler}
server := http.Server{
Addr: s.address,
Handler: s.handler,
ReadHeaderTimeout: s.readHeaderTimeout,
ReadTimeout: s.readTimeout,
}
crashed := make(chan struct{})
shutdownDone := make(chan struct{})

View File

@@ -9,11 +9,13 @@ import (
// Server is an HTTP server implementation, which uses
// the HTTP handler provided.
type Server struct {
address string
addressSet chan struct{}
handler http.Handler
logger Logger
shutdownTimeout time.Duration
address string
addressSet chan struct{}
handler http.Handler
logger Logger
readHeaderTimeout time.Duration
readTimeout time.Duration
shutdownTimeout time.Duration
}
// New creates a new HTTP server with the given settings.
@@ -26,10 +28,12 @@ func New(settings Settings) (s *Server, err error) {
}
return &Server{
address: settings.Address,
addressSet: make(chan struct{}),
handler: settings.Handler,
logger: settings.Logger,
shutdownTimeout: *settings.ShutdownTimeout,
address: settings.Address,
addressSet: make(chan struct{}),
handler: settings.Handler,
logger: settings.Logger,
readHeaderTimeout: settings.ReadHeaderTimeout,
readTimeout: settings.ReadTimeout,
shutdownTimeout: settings.ShutdownTimeout,
}, nil
}

View File

@@ -29,16 +29,20 @@ func Test_New(t *testing.T) {
},
"filled settings": {
settings: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: &Server{
address: ":8001",
handler: someHandler,
logger: someLogger,
shutdownTimeout: time.Second,
address: ":8001",
handler: someHandler,
logger: someLogger,
readHeaderTimeout: time.Second,
readTimeout: time.Second,
shutdownTimeout: time.Second,
},
},
}

View File

@@ -22,23 +22,34 @@ type Settings struct {
// Logger is the logger to use.
// It must be set and cannot be left to nil.
Logger Logger
// ReadHeaderTimeout is the HTTP header read timeout duration
// of the HTTP server. It defaults to 3 seconds if left unset.
ReadHeaderTimeout time.Duration
// ReadTimeout is the HTTP read timeout duration
// of the HTTP server. It defaults to 3 seconds if left unset.
ReadTimeout time.Duration
// ShutdownTimeout is the shutdown timeout duration
// of the HTTP server. It defaults to 3 seconds.
ShutdownTimeout *time.Duration
// of the HTTP server. It defaults to 3 seconds if left unset.
ShutdownTimeout time.Duration
}
func (s *Settings) SetDefaults() {
s.Address = helpers.DefaultString(s.Address, ":8000")
const defaultReadTimeout = 3 * time.Second
s.ReadHeaderTimeout = helpers.DefaultDuration(s.ReadHeaderTimeout, defaultReadTimeout)
s.ReadTimeout = helpers.DefaultDuration(s.ReadTimeout, defaultReadTimeout)
const defaultShutdownTimeout = 3 * time.Second
s.ShutdownTimeout = helpers.DefaultDuration(s.ShutdownTimeout, defaultShutdownTimeout)
}
func (s Settings) Copy() Settings {
return Settings{
Address: s.Address,
Handler: s.Handler,
Logger: s.Logger,
ShutdownTimeout: helpers.CopyDurationPtr(s.ShutdownTimeout),
Address: s.Address,
Handler: s.Handler,
Logger: s.Logger,
ReadHeaderTimeout: s.ReadHeaderTimeout,
ReadTimeout: s.ReadTimeout,
ShutdownTimeout: s.ShutdownTimeout,
}
}
@@ -48,6 +59,8 @@ func (s *Settings) MergeWith(other Settings) {
if s.Logger == nil {
s.Logger = other.Logger
}
s.ReadHeaderTimeout = helpers.MergeWithDuration(s.ReadHeaderTimeout, other.ReadHeaderTimeout)
s.ReadTimeout = helpers.MergeWithDuration(s.ReadTimeout, other.ReadTimeout)
s.ShutdownTimeout = helpers.MergeWithDuration(s.ShutdownTimeout, other.ShutdownTimeout)
}
@@ -57,13 +70,17 @@ func (s *Settings) OverrideWith(other Settings) {
if other.Logger != nil {
s.Logger = other.Logger
}
s.ReadHeaderTimeout = helpers.OverrideWithDuration(s.ReadHeaderTimeout, other.ReadHeaderTimeout)
s.ReadTimeout = helpers.OverrideWithDuration(s.ReadTimeout, other.ReadTimeout)
s.ShutdownTimeout = helpers.OverrideWithDuration(s.ShutdownTimeout, other.ShutdownTimeout)
}
var (
ErrHandlerIsNotSet = errors.New("HTTP handler cannot be left unset")
ErrLoggerIsNotSet = errors.New("logger cannot be left unset")
ErrShutdownTimeoutTooSmall = errors.New("shutdown timeout is too small")
ErrHandlerIsNotSet = errors.New("HTTP handler cannot be left unset")
ErrLoggerIsNotSet = errors.New("logger cannot be left unset")
ErrReadHeaderTimeoutTooSmall = errors.New("read header timeout is too small")
ErrReadTimeoutTooSmall = errors.New("read timeout is too small")
ErrShutdownTimeoutTooSmall = errors.New("shutdown timeout is too small")
)
func (s Settings) Validate() (err error) {
@@ -81,11 +98,24 @@ func (s Settings) Validate() (err error) {
return ErrLoggerIsNotSet
}
const minReadTimeout = time.Millisecond
if s.ReadHeaderTimeout < minReadTimeout {
return fmt.Errorf("%w: %s must be at least %s",
ErrReadHeaderTimeoutTooSmall,
s.ReadHeaderTimeout, minReadTimeout)
}
if s.ReadTimeout < minReadTimeout {
return fmt.Errorf("%w: %s must be at least %s",
ErrReadTimeoutTooSmall,
s.ReadTimeout, minReadTimeout)
}
const minShutdownTimeout = 5 * time.Millisecond
if *s.ShutdownTimeout < minShutdownTimeout {
if s.ShutdownTimeout < minShutdownTimeout {
return fmt.Errorf("%w: %s must be at least %s",
ErrShutdownTimeoutTooSmall,
*s.ShutdownTimeout, minShutdownTimeout)
s.ShutdownTimeout, minShutdownTimeout)
}
return nil
@@ -94,7 +124,9 @@ func (s Settings) Validate() (err error) {
func (s Settings) ToLinesNode() (node *gotree.Node) {
node = gotree.New("HTTP server settings:")
node.Appendf("Listening address: %s", s.Address)
node.Appendf("Shutdown timeout: %s", *s.ShutdownTimeout)
node.Appendf("Read header timeout: %s", s.ReadHeaderTimeout)
node.Appendf("Read timeout: %s", s.ReadTimeout)
node.Appendf("Shutdown timeout: %s", s.ShutdownTimeout)
return node
}

View File

@@ -21,18 +21,24 @@ func Test_Settings_SetDefaults(t *testing.T) {
"empty settings": {
settings: Settings{},
expected: Settings{
Address: ":8000",
ShutdownTimeout: durationPtr(defaultTimeout),
Address: ":8000",
ReadHeaderTimeout: defaultTimeout,
ReadTimeout: defaultTimeout,
ShutdownTimeout: defaultTimeout,
},
},
"filled settings": {
settings: Settings{
Address: ":8001",
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
}
@@ -62,16 +68,20 @@ func Test_Settings_Copy(t *testing.T) {
"empty settings": {},
"filled settings": {
settings: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
}
@@ -102,30 +112,38 @@ func Test_Settings_MergeWith(t *testing.T) {
"merge empty with empty": {},
"merge empty with filled": {
other: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
"merge filled with empty": {
settings: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
}
@@ -156,48 +174,62 @@ func Test_Settings_OverrideWith(t *testing.T) {
"override empty with empty": {},
"override empty with filled": {
other: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
"override filled with empty": {
settings: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
expected: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
},
"override filled with filled": {
settings: Settings{
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8001",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Second,
ReadTimeout: time.Second,
ShutdownTimeout: time.Second,
},
other: Settings{
Address: ":8002",
ShutdownTimeout: durationPtr(time.Hour),
Address: ":8002",
ReadHeaderTimeout: time.Hour,
ReadTimeout: time.Hour,
ShutdownTimeout: time.Hour,
},
expected: Settings{
Address: ":8002",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Hour),
Address: ":8002",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Hour,
ReadTimeout: time.Hour,
ShutdownTimeout: time.Hour,
},
},
}
@@ -247,22 +279,47 @@ func Test_Settings_Validate(t *testing.T) {
errWrapped: ErrLoggerIsNotSet,
errMessage: ErrLoggerIsNotSet.Error(),
},
"read header timeout too small": {
settings: Settings{
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Nanosecond,
},
errWrapped: ErrReadHeaderTimeoutTooSmall,
errMessage: "read header timeout is too small: 1ns must be at least 1ms",
},
"read timeout too small": {
settings: Settings{
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Millisecond,
ReadTimeout: time.Nanosecond,
},
errWrapped: ErrReadTimeoutTooSmall,
errMessage: "read timeout is too small: 1ns must be at least 1ms",
},
"shutdown timeout too small": {
settings: Settings{
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Millisecond),
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Millisecond,
ReadTimeout: time.Millisecond,
ShutdownTimeout: time.Millisecond,
},
errWrapped: ErrShutdownTimeoutTooSmall,
errMessage: "shutdown timeout is too small: 1ms must be at least 5ms",
},
"valid settings": {
settings: Settings{
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ShutdownTimeout: durationPtr(time.Second),
Address: ":8000",
Handler: someHandler,
Logger: someLogger,
ReadHeaderTimeout: time.Millisecond,
ReadTimeout: time.Millisecond,
ShutdownTimeout: time.Second,
},
},
}
@@ -291,11 +348,15 @@ func Test_Settings_String(t *testing.T) {
}{
"all values": {
settings: Settings{
Address: ":8000",
ShutdownTimeout: durationPtr(time.Second),
Address: ":8000",
ReadHeaderTimeout: time.Millisecond,
ReadTimeout: time.Millisecond,
ShutdownTimeout: time.Second,
},
s: `HTTP server settings:
├── Listening address: :8000
├── Read header timeout: 1ms
├── Read timeout: 1ms
└── Shutdown timeout: 1s`,
},
}