diff --git a/Dockerfile b/Dockerfile index 5c840214..040c57b9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ ARG ALPINE_VERSION=3.16 ARG GO_ALPINE_VERSION=3.16 ARG GO_VERSION=1.17 ARG XCPUTRANSLATE_VERSION=v0.6.0 -ARG GOLANGCI_LINT_VERSION=v1.46.2 +ARG GOLANGCI_LINT_VERSION=v1.47.2 ARG MOCKGEN_VERSION=v1.6.0 ARG BUILDPLATFORM=linux/amd64 diff --git a/internal/configuration/settings/dot.go b/internal/configuration/settings/dot.go index bd0ed047..ae2db543 100644 --- a/internal/configuration/settings/dot.go +++ b/internal/configuration/settings/dot.go @@ -65,7 +65,7 @@ func (d *DoT) copy() (copied DoT) { // unset field of the receiver settings object. func (d *DoT) mergeWith(other DoT) { d.Enabled = helpers.MergeWithBool(d.Enabled, other.Enabled) - d.UpdatePeriod = helpers.MergeWithDuration(d.UpdatePeriod, other.UpdatePeriod) + d.UpdatePeriod = helpers.MergeWithDurationPtr(d.UpdatePeriod, other.UpdatePeriod) d.Unbound.mergeWith(other.Unbound) d.Blacklist.mergeWith(other.Blacklist) } @@ -75,7 +75,7 @@ func (d *DoT) mergeWith(other DoT) { // settings. func (d *DoT) overrideWith(other DoT) { d.Enabled = helpers.OverrideWithBool(d.Enabled, other.Enabled) - d.UpdatePeriod = helpers.OverrideWithDuration(d.UpdatePeriod, other.UpdatePeriod) + d.UpdatePeriod = helpers.OverrideWithDurationPtr(d.UpdatePeriod, other.UpdatePeriod) d.Unbound.overrideWith(other.Unbound) d.Blacklist.overrideWith(other.Blacklist) } @@ -83,7 +83,7 @@ func (d *DoT) overrideWith(other DoT) { func (d *DoT) setDefaults() { d.Enabled = helpers.DefaultBool(d.Enabled, true) const defaultUpdatePeriod = 24 * time.Hour - d.UpdatePeriod = helpers.DefaultDuration(d.UpdatePeriod, defaultUpdatePeriod) + d.UpdatePeriod = helpers.DefaultDurationPtr(d.UpdatePeriod, defaultUpdatePeriod) d.Unbound.setDefaults() d.Blacklist.setDefaults() } diff --git a/internal/configuration/settings/health.go b/internal/configuration/settings/health.go index 7ff45b36..468f3865 100644 --- a/internal/configuration/settings/health.go +++ b/internal/configuration/settings/health.go @@ -3,6 +3,7 @@ package settings import ( "fmt" "os" + "time" "github.com/qdm12/gluetun/internal/configuration/settings/helpers" "github.com/qdm12/gotree" @@ -15,6 +16,12 @@ type Health struct { // for the health check server. // It cannot be the empty string in the internal state. ServerAddress string + // ReadHeaderTimeout is the HTTP server header read timeout + // duration of the HTTP server. It defaults to 100 milliseconds. + ReadHeaderTimeout time.Duration + // ReadTimeout is the HTTP read timeout duration of the + // HTTP server. It defaults to 500 milliseconds. + ReadTimeout time.Duration // TargetAddress is the address (host or host:port) // to TCP dial to periodically for the health check. // It cannot be the empty string in the internal state. @@ -40,9 +47,11 @@ func (h Health) Validate() (err error) { func (h *Health) copy() (copied Health) { return Health{ - ServerAddress: h.ServerAddress, - TargetAddress: h.TargetAddress, - VPN: h.VPN.copy(), + ServerAddress: h.ServerAddress, + ReadHeaderTimeout: h.ReadHeaderTimeout, + ReadTimeout: h.ReadTimeout, + TargetAddress: h.TargetAddress, + VPN: h.VPN.copy(), } } @@ -50,6 +59,8 @@ func (h *Health) copy() (copied Health) { // unset field of the receiver settings object. func (h *Health) MergeWith(other Health) { h.ServerAddress = helpers.MergeWithString(h.ServerAddress, other.ServerAddress) + h.ReadHeaderTimeout = helpers.MergeWithDuration(h.ReadHeaderTimeout, other.ReadHeaderTimeout) + h.ReadTimeout = helpers.MergeWithDuration(h.ReadTimeout, other.ReadTimeout) h.TargetAddress = helpers.MergeWithString(h.TargetAddress, other.TargetAddress) h.VPN.mergeWith(other.VPN) } @@ -59,12 +70,18 @@ func (h *Health) MergeWith(other Health) { // settings. func (h *Health) OverrideWith(other Health) { h.ServerAddress = helpers.OverrideWithString(h.ServerAddress, other.ServerAddress) + h.ReadHeaderTimeout = helpers.OverrideWithDuration(h.ReadHeaderTimeout, other.ReadHeaderTimeout) + h.ReadTimeout = helpers.OverrideWithDuration(h.ReadTimeout, other.ReadTimeout) h.TargetAddress = helpers.OverrideWithString(h.TargetAddress, other.TargetAddress) h.VPN.overrideWith(other.VPN) } func (h *Health) SetDefaults() { h.ServerAddress = helpers.DefaultString(h.ServerAddress, "127.0.0.1:9999") + const defaultReadHeaderTimeout = 100 * time.Millisecond + h.ReadHeaderTimeout = helpers.DefaultDuration(h.ReadHeaderTimeout, defaultReadHeaderTimeout) + const defaultReadTimeout = 500 * time.Millisecond + h.ReadTimeout = helpers.DefaultDuration(h.ReadTimeout, defaultReadTimeout) h.TargetAddress = helpers.DefaultString(h.TargetAddress, "cloudflare.com:443") h.VPN.setDefaults() } @@ -77,6 +94,8 @@ func (h Health) toLinesNode() (node *gotree.Node) { node = gotree.New("Health settings:") node.Appendf("Server listening address: %s", h.ServerAddress) node.Appendf("Target address: %s", h.TargetAddress) + node.Appendf("Read header timeout: %s", h.ReadHeaderTimeout) + node.Appendf("Read timeout: %s", h.ReadTimeout) node.AppendNode(h.VPN.toLinesNode("VPN")) return node } diff --git a/internal/configuration/settings/healthywait.go b/internal/configuration/settings/healthywait.go index 0d397735..bf3acb31 100644 --- a/internal/configuration/settings/healthywait.go +++ b/internal/configuration/settings/healthywait.go @@ -35,23 +35,23 @@ func (h *HealthyWait) copy() (copied HealthyWait) { // mergeWith merges the other settings into any // unset field of the receiver settings object. func (h *HealthyWait) mergeWith(other HealthyWait) { - h.Initial = helpers.MergeWithDuration(h.Initial, other.Initial) - h.Addition = helpers.MergeWithDuration(h.Addition, other.Addition) + h.Initial = helpers.MergeWithDurationPtr(h.Initial, other.Initial) + h.Addition = helpers.MergeWithDurationPtr(h.Addition, other.Addition) } // overrideWith overrides fields of the receiver // settings object with any field set in the other // settings. func (h *HealthyWait) overrideWith(other HealthyWait) { - h.Initial = helpers.OverrideWithDuration(h.Initial, other.Initial) - h.Addition = helpers.OverrideWithDuration(h.Addition, other.Addition) + h.Initial = helpers.OverrideWithDurationPtr(h.Initial, other.Initial) + h.Addition = helpers.OverrideWithDurationPtr(h.Addition, other.Addition) } func (h *HealthyWait) setDefaults() { const initialDurationDefault = 6 * time.Second const additionDurationDefault = 5 * time.Second - h.Initial = helpers.DefaultDuration(h.Initial, initialDurationDefault) - h.Addition = helpers.DefaultDuration(h.Addition, additionDurationDefault) + h.Initial = helpers.DefaultDurationPtr(h.Initial, initialDurationDefault) + h.Addition = helpers.DefaultDurationPtr(h.Addition, additionDurationDefault) } func (h HealthyWait) String() string { diff --git a/internal/configuration/settings/helpers/default.go b/internal/configuration/settings/helpers/default.go index 0951a5c1..9c2fa07c 100644 --- a/internal/configuration/settings/helpers/default.go +++ b/internal/configuration/settings/helpers/default.go @@ -73,7 +73,15 @@ func DefaultStringPtr(existing *string, defaultValue string) (result *string) { return result } -func DefaultDuration(existing *time.Duration, +func DefaultDuration(existing time.Duration, + defaultValue time.Duration) (result time.Duration) { + if existing != 0 { + return existing + } + return defaultValue +} + +func DefaultDurationPtr(existing *time.Duration, defaultValue time.Duration) (result *time.Duration) { if existing != nil { return existing diff --git a/internal/configuration/settings/helpers/merge.go b/internal/configuration/settings/helpers/merge.go index e1afd670..4b96218d 100644 --- a/internal/configuration/settings/helpers/merge.go +++ b/internal/configuration/settings/helpers/merge.go @@ -107,7 +107,14 @@ func MergeWithIP(existing, other net.IP) (result net.IP) { return result } -func MergeWithDuration(existing, other *time.Duration) (result *time.Duration) { +func MergeWithDuration(existing, other time.Duration) (result time.Duration) { + if existing != 0 { + return existing + } + return other +} + +func MergeWithDurationPtr(existing, other *time.Duration) (result *time.Duration) { if existing != nil { return existing } diff --git a/internal/configuration/settings/helpers/override.go b/internal/configuration/settings/helpers/override.go index bfad029a..49c5ddf2 100644 --- a/internal/configuration/settings/helpers/override.go +++ b/internal/configuration/settings/helpers/override.go @@ -93,7 +93,16 @@ func OverrideWithIP(existing, other net.IP) (result net.IP) { return result } -func OverrideWithDuration(existing, other *time.Duration) (result *time.Duration) { +func OverrideWithDuration(existing, other time.Duration) ( + result time.Duration) { + if other == 0 { + return existing + } + return other +} + +func OverrideWithDurationPtr(existing, other *time.Duration) ( + result *time.Duration) { if other == nil { return existing } diff --git a/internal/configuration/settings/httpproxy.go b/internal/configuration/settings/httpproxy.go index 59adb046..31823442 100644 --- a/internal/configuration/settings/httpproxy.go +++ b/internal/configuration/settings/httpproxy.go @@ -3,6 +3,7 @@ package settings import ( "fmt" "os" + "time" "github.com/qdm12/gluetun/internal/configuration/settings/helpers" "github.com/qdm12/gotree" @@ -33,6 +34,12 @@ type HTTPProxy struct { // each request/response. It cannot be nil in the // internal state. Log *bool + // ReadHeaderTimeout is the HTTP header read timeout duration + // of the HTTP server. It defaults to 1 second 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 } func (h HTTPProxy) validate() (err error) { @@ -49,12 +56,14 @@ func (h HTTPProxy) validate() (err error) { func (h *HTTPProxy) copy() (copied HTTPProxy) { return HTTPProxy{ - User: helpers.CopyStringPtr(h.User), - Password: helpers.CopyStringPtr(h.Password), - ListeningAddress: h.ListeningAddress, - Enabled: helpers.CopyBoolPtr(h.Enabled), - Stealth: helpers.CopyBoolPtr(h.Stealth), - Log: helpers.CopyBoolPtr(h.Log), + User: helpers.CopyStringPtr(h.User), + Password: helpers.CopyStringPtr(h.Password), + ListeningAddress: h.ListeningAddress, + Enabled: helpers.CopyBoolPtr(h.Enabled), + Stealth: helpers.CopyBoolPtr(h.Stealth), + Log: helpers.CopyBoolPtr(h.Log), + ReadHeaderTimeout: h.ReadHeaderTimeout, + ReadTimeout: h.ReadTimeout, } } @@ -67,6 +76,8 @@ func (h *HTTPProxy) mergeWith(other HTTPProxy) { h.Enabled = helpers.MergeWithBool(h.Enabled, other.Enabled) h.Stealth = helpers.MergeWithBool(h.Stealth, other.Stealth) h.Log = helpers.MergeWithBool(h.Log, other.Log) + h.ReadHeaderTimeout = helpers.MergeWithDuration(h.ReadHeaderTimeout, other.ReadHeaderTimeout) + h.ReadTimeout = helpers.MergeWithDuration(h.ReadTimeout, other.ReadTimeout) } // overrideWith overrides fields of the receiver @@ -79,6 +90,8 @@ func (h *HTTPProxy) overrideWith(other HTTPProxy) { h.Enabled = helpers.OverrideWithBool(h.Enabled, other.Enabled) h.Stealth = helpers.OverrideWithBool(h.Stealth, other.Stealth) h.Log = helpers.OverrideWithBool(h.Log, other.Log) + h.ReadHeaderTimeout = helpers.OverrideWithDuration(h.ReadHeaderTimeout, other.ReadHeaderTimeout) + h.ReadTimeout = helpers.OverrideWithDuration(h.ReadTimeout, other.ReadTimeout) } func (h *HTTPProxy) setDefaults() { @@ -88,6 +101,10 @@ func (h *HTTPProxy) setDefaults() { h.Enabled = helpers.DefaultBool(h.Enabled, false) h.Stealth = helpers.DefaultBool(h.Stealth, false) h.Log = helpers.DefaultBool(h.Log, false) + const defaultReadHeaderTimeout = time.Second + h.ReadHeaderTimeout = helpers.DefaultDuration(h.ReadHeaderTimeout, defaultReadHeaderTimeout) + const defaultReadTimeout = 3 * time.Second + h.ReadTimeout = helpers.DefaultDuration(h.ReadTimeout, defaultReadTimeout) } func (h HTTPProxy) String() string { @@ -106,6 +123,8 @@ func (h HTTPProxy) toLinesNode() (node *gotree.Node) { node.Appendf("Password: %s", helpers.ObfuscatePassword(*h.Password)) node.Appendf("Stealth mode: %s", helpers.BoolPtrToYesNo(h.Stealth)) node.Appendf("Log: %s", helpers.BoolPtrToYesNo(h.Log)) + node.Appendf("Read header timeout: %s", h.ReadHeaderTimeout) + node.Appendf("Read timeout: %s", h.ReadTimeout) return node } diff --git a/internal/configuration/settings/publicip.go b/internal/configuration/settings/publicip.go index 10780f70..5f33ff90 100644 --- a/internal/configuration/settings/publicip.go +++ b/internal/configuration/settings/publicip.go @@ -48,18 +48,18 @@ func (p *PublicIP) copy() (copied PublicIP) { } func (p *PublicIP) mergeWith(other PublicIP) { - p.Period = helpers.MergeWithDuration(p.Period, other.Period) + p.Period = helpers.MergeWithDurationPtr(p.Period, other.Period) p.IPFilepath = helpers.MergeWithStringPtr(p.IPFilepath, other.IPFilepath) } func (p *PublicIP) overrideWith(other PublicIP) { - p.Period = helpers.OverrideWithDuration(p.Period, other.Period) + p.Period = helpers.OverrideWithDurationPtr(p.Period, other.Period) p.IPFilepath = helpers.OverrideWithStringPtr(p.IPFilepath, other.IPFilepath) } func (p *PublicIP) setDefaults() { const defaultPeriod = 12 * time.Hour - p.Period = helpers.DefaultDuration(p.Period, defaultPeriod) + p.Period = helpers.DefaultDurationPtr(p.Period, defaultPeriod) p.IPFilepath = helpers.DefaultStringPtr(p.IPFilepath, "/tmp/gluetun/ip") } diff --git a/internal/configuration/settings/settings_test.go b/internal/configuration/settings/settings_test.go index 0238e446..0ba4ae0b 100644 --- a/internal/configuration/settings/settings_test.go +++ b/internal/configuration/settings/settings_test.go @@ -67,6 +67,8 @@ func Test_Settings_String(t *testing.T) { ├── Health settings: | ├── Server listening address: 127.0.0.1:9999 | ├── Target address: cloudflare.com:443 +| ├── Read header timeout: 100ms +| ├── Read timeout: 500ms | └── VPN wait durations: | ├── Initial duration: 6s | └── Additional duration: 5s diff --git a/internal/configuration/settings/updater.go b/internal/configuration/settings/updater.go index 2628f380..8b7a2d69 100644 --- a/internal/configuration/settings/updater.go +++ b/internal/configuration/settings/updater.go @@ -73,7 +73,7 @@ func (u *Updater) copy() (copied Updater) { // mergeWith merges the other settings into any // unset field of the receiver settings object. func (u *Updater) mergeWith(other Updater) { - u.Period = helpers.MergeWithDuration(u.Period, other.Period) + u.Period = helpers.MergeWithDurationPtr(u.Period, other.Period) u.DNSAddress = helpers.MergeWithString(u.DNSAddress, other.DNSAddress) u.MinRatio = helpers.MergeWithFloat64(u.MinRatio, other.MinRatio) u.Providers = helpers.MergeStringSlices(u.Providers, other.Providers) @@ -83,14 +83,14 @@ func (u *Updater) mergeWith(other Updater) { // settings object with any field set in the other // settings. func (u *Updater) overrideWith(other Updater) { - u.Period = helpers.OverrideWithDuration(u.Period, other.Period) + u.Period = helpers.OverrideWithDurationPtr(u.Period, other.Period) u.DNSAddress = helpers.OverrideWithString(u.DNSAddress, other.DNSAddress) u.MinRatio = helpers.OverrideWithFloat64(u.MinRatio, other.MinRatio) u.Providers = helpers.OverrideWithStringSlice(u.Providers, other.Providers) } func (u *Updater) SetDefaults(vpnProvider string) { - u.Period = helpers.DefaultDuration(u.Period, 0) + u.Period = helpers.DefaultDurationPtr(u.Period, 0) u.DNSAddress = helpers.DefaultString(u.DNSAddress, "1.1.1.1:53") if u.MinRatio == 0 { diff --git a/internal/healthcheck/run.go b/internal/healthcheck/run.go index be5418a2..5f7bb7fc 100644 --- a/internal/healthcheck/run.go +++ b/internal/healthcheck/run.go @@ -14,8 +14,10 @@ func (s *Server) Run(ctx context.Context, done chan<- struct{}) { go s.runHealthcheckLoop(ctx, loopDone) server := http.Server{ - Addr: s.config.ServerAddress, - Handler: s.handler, + Addr: s.config.ServerAddress, + Handler: s.handler, + ReadHeaderTimeout: s.config.ReadHeaderTimeout, + ReadTimeout: s.config.ReadTimeout, } serverDone := make(chan struct{}) go func() { diff --git a/internal/httpproxy/run.go b/internal/httpproxy/run.go index 4248c9c8..6831d5d7 100644 --- a/internal/httpproxy/run.go +++ b/internal/httpproxy/run.go @@ -23,7 +23,7 @@ func (l *Loop) Run(ctx context.Context, done chan<- struct{}) { settings := l.state.GetSettings() server := New(runCtx, settings.ListeningAddress, l.logger, *settings.Stealth, *settings.Log, *settings.User, - *settings.Password) + *settings.Password, settings.ReadHeaderTimeout, settings.ReadTimeout) errorCh := make(chan error) go server.Run(runCtx, errorCh) diff --git a/internal/httpproxy/server.go b/internal/httpproxy/server.go index 2c73620d..7f000d84 100644 --- a/internal/httpproxy/server.go +++ b/internal/httpproxy/server.go @@ -8,25 +8,35 @@ import ( ) type Server struct { - address string - handler http.Handler - logger infoErrorer - internalWG *sync.WaitGroup + address string + handler http.Handler + logger infoErrorer + internalWG *sync.WaitGroup + readHeaderTimeout time.Duration + readTimeout time.Duration } func New(ctx context.Context, address string, logger Logger, - stealth, verbose bool, username, password string) *Server { + stealth, verbose bool, username, password string, + readHeaderTimeout, readTimeout time.Duration) *Server { wg := &sync.WaitGroup{} return &Server{ - address: address, - handler: newHandler(ctx, wg, logger, stealth, verbose, username, password), - logger: logger, - internalWG: wg, + address: address, + handler: newHandler(ctx, wg, logger, stealth, verbose, username, password), + logger: logger, + internalWG: wg, + readHeaderTimeout: readHeaderTimeout, + readTimeout: readTimeout, } } func (s *Server) Run(ctx context.Context, errorCh chan<- error) { - server := http.Server{Addr: s.address, Handler: s.handler} + server := http.Server{ + Addr: s.address, + Handler: s.handler, + ReadHeaderTimeout: s.readHeaderTimeout, + ReadTimeout: s.readTimeout, + } go func() { <-ctx.Done() const shutdownGraceDuration = 2 * time.Second diff --git a/internal/httpserver/helpers_test.go b/internal/httpserver/helpers_test.go index c17fa859..a0286bb6 100644 --- a/internal/httpserver/helpers_test.go +++ b/internal/httpserver/helpers_test.go @@ -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{} diff --git a/internal/httpserver/run.go b/internal/httpserver/run.go index 55608287..4d2cc106 100644 --- a/internal/httpserver/run.go +++ b/internal/httpserver/run.go @@ -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{}) diff --git a/internal/httpserver/server.go b/internal/httpserver/server.go index 8c6d4dfa..b703b236 100644 --- a/internal/httpserver/server.go +++ b/internal/httpserver/server.go @@ -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 } diff --git a/internal/httpserver/server_test.go b/internal/httpserver/server_test.go index 330b9af4..e8654009 100644 --- a/internal/httpserver/server_test.go +++ b/internal/httpserver/server_test.go @@ -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, }, }, } diff --git a/internal/httpserver/settings.go b/internal/httpserver/settings.go index e0bd0227..1c4801e5 100644 --- a/internal/httpserver/settings.go +++ b/internal/httpserver/settings.go @@ -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 } diff --git a/internal/httpserver/settings_test.go b/internal/httpserver/settings_test.go index 8f4bdb1f..331ca91d 100644 --- a/internal/httpserver/settings_test.go +++ b/internal/httpserver/settings_test.go @@ -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`, }, } diff --git a/internal/pprof/helpers_test.go b/internal/pprof/helpers_test.go index f74e6d6a..2e34f7c6 100644 --- a/internal/pprof/helpers_test.go +++ b/internal/pprof/helpers_test.go @@ -2,13 +2,11 @@ package pprof import ( "regexp" - "time" gomock "github.com/golang/mock/gomock" ) -func boolPtr(b bool) *bool { return &b } -func durationPtr(d time.Duration) *time.Duration { return &d } +func boolPtr(b bool) *bool { return &b } var _ gomock.Matcher = (*regexMatcher)(nil) diff --git a/internal/pprof/server_test.go b/internal/pprof/server_test.go index 80fd4df9..e5db82ff 100644 --- a/internal/pprof/server_test.go +++ b/internal/pprof/server_test.go @@ -29,7 +29,7 @@ func Test_Server(t *testing.T) { HTTPServer: httpserver.Settings{ Address: address, Logger: logger, - ShutdownTimeout: durationPtr(httpServerShutdownTimeout), + ShutdownTimeout: httpServerShutdownTimeout, }, } diff --git a/internal/pprof/settings.go b/internal/pprof/settings.go index a0d1bff4..40f9faea 100644 --- a/internal/pprof/settings.go +++ b/internal/pprof/settings.go @@ -2,6 +2,7 @@ package pprof import ( "errors" + "time" "github.com/qdm12/gluetun/internal/configuration/settings/helpers" "github.com/qdm12/gluetun/internal/httpserver" @@ -27,6 +28,8 @@ type Settings struct { func (s *Settings) SetDefaults() { s.Enabled = helpers.DefaultBool(s.Enabled, false) s.HTTPServer.Address = helpers.DefaultString(s.HTTPServer.Address, "localhost:6060") + const defaultReadTimeout = 5 * time.Minute // for CPU profiling + s.HTTPServer.ReadTimeout = helpers.DefaultDuration(s.HTTPServer.ReadTimeout, defaultReadTimeout) s.HTTPServer.SetDefaults() } diff --git a/internal/pprof/settings_test.go b/internal/pprof/settings_test.go index 0ae9d155..b229e8ab 100644 --- a/internal/pprof/settings_test.go +++ b/internal/pprof/settings_test.go @@ -21,8 +21,10 @@ func Test_Settings_SetDefaults(t *testing.T) { expected: Settings{ Enabled: boolPtr(false), HTTPServer: httpserver.Settings{ - Address: "localhost:6060", - ShutdownTimeout: durationPtr(3 * time.Second), + Address: "localhost:6060", + ReadHeaderTimeout: 3 * time.Second, + ReadTimeout: 5 * time.Minute, + ShutdownTimeout: 3 * time.Second, }, }, }, @@ -32,8 +34,10 @@ func Test_Settings_SetDefaults(t *testing.T) { BlockProfileRate: 1, MutexProfileRate: 1, HTTPServer: httpserver.Settings{ - Address: ":6061", - ShutdownTimeout: durationPtr(time.Second), + Address: ":6061", + ReadHeaderTimeout: time.Second, + ReadTimeout: time.Second, + ShutdownTimeout: time.Second, }, }, expected: Settings{ @@ -41,8 +45,10 @@ func Test_Settings_SetDefaults(t *testing.T) { BlockProfileRate: 1, MutexProfileRate: 1, HTTPServer: httpserver.Settings{ - Address: ":6061", - ShutdownTimeout: durationPtr(time.Second), + Address: ":6061", + ReadHeaderTimeout: time.Second, + ReadTimeout: time.Second, + ShutdownTimeout: time.Second, }, }, }, @@ -75,7 +81,7 @@ func Test_Settings_Copy(t *testing.T) { MutexProfileRate: 1, HTTPServer: httpserver.Settings{ Address: ":6061", - ShutdownTimeout: durationPtr(time.Second), + ShutdownTimeout: time.Second, }, }, expected: Settings{ @@ -84,7 +90,7 @@ func Test_Settings_Copy(t *testing.T) { MutexProfileRate: 1, HTTPServer: httpserver.Settings{ Address: ":6061", - ShutdownTimeout: durationPtr(time.Second), + ShutdownTimeout: time.Second, }, }, }, @@ -278,10 +284,12 @@ func Test_Settings_Validate(t *testing.T) { "valid settings": { settings: Settings{ HTTPServer: httpserver.Settings{ - Address: ":8000", - Handler: http.NewServeMux(), - Logger: &MockLogger{}, - ShutdownTimeout: durationPtr(time.Second), + Address: ":8000", + Handler: http.NewServeMux(), + Logger: &MockLogger{}, + ReadHeaderTimeout: time.Second, + ReadTimeout: time.Second, + ShutdownTimeout: time.Second, }, }, }, @@ -321,7 +329,7 @@ func Test_Settings_String(t *testing.T) { MutexProfileRate: 1, HTTPServer: httpserver.Settings{ Address: ":8000", - ShutdownTimeout: durationPtr(time.Second), + ShutdownTimeout: time.Second, }, }, s: `Pprof settings: @@ -329,6 +337,8 @@ func Test_Settings_String(t *testing.T) { ├── Mutex profile rate: 1 └── HTTP server settings: ├── Listening address: :8000 + ├── Read header timeout: 0s + ├── Read timeout: 0s └── Shutdown timeout: 1s`, }, }