From 99c1461025c10f631b2d58cfefb8b87204733029 Mon Sep 17 00:00:00 2001 From: guscarreon Date: Mon, 22 Apr 2024 12:59:03 -0400 Subject: [PATCH 1/5] Config max request header size --- config/config.go | 8 ++++++++ config/config_test.go | 2 ++ config/configtest/sample_full_config.yaml | 1 + server/server.go | 20 ++++++++++++++++++-- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 59e02080..2821cc95 100644 --- a/config/config.go +++ b/config/config.go @@ -95,6 +95,7 @@ func setConfigDefaults(v *viper.Viper) { v.SetDefault("request_limits.max_size_bytes", utils.REQUEST_MAX_SIZE_BYTES) v.SetDefault("request_limits.max_num_values", utils.REQUEST_MAX_NUM_VALUES) v.SetDefault("request_limits.max_ttl_seconds", utils.REQUEST_MAX_TTL_SECONDS) + v.SetDefault("request_limits.max_header_size_bytes", 0) v.SetDefault("routes.allow_public_write", true) } @@ -179,6 +180,7 @@ type RequestLimits struct { MaxNumValues int `mapstructure:"max_num_values"` MaxTTLSeconds int `mapstructure:"max_ttl_seconds"` AllowSettingKeys bool `mapstructure:"allow_setting_keys"` + MaxHeaderSize int `mapstructure:"max_header_size_bytes"` } func (cfg *RequestLimits) validateAndLog() { @@ -201,6 +203,12 @@ func (cfg *RequestLimits) validateAndLog() { } else { log.Fatalf("invalid config.request_limits.max_num_values: %d. Value cannot be negative.", cfg.MaxNumValues) } + + if cfg.MaxHeaderSize >= 0 { + log.Infof("config.request_limits.max_header_size_bytes: %d", cfg.MaxHeaderSize) + } else { + log.Fatalf("invalid config.request_limits.max_header_size_bytes: %d. Value cannot be negative.", cfg.MaxHeaderSize) + } } type Compression struct { diff --git a/config/config_test.go b/config/config_test.go index 8ab6da94..6ac4e579 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1078,6 +1078,7 @@ func TestConfigurationValidateAndLog(t *testing.T) { {msg: fmt.Sprintf("config.request_limits.max_ttl_seconds: %d", expectedConfig.RequestLimits.MaxTTLSeconds), lvl: logrus.InfoLevel}, {msg: fmt.Sprintf("config.request_limits.max_size_bytes: %d", expectedConfig.RequestLimits.MaxSize), lvl: logrus.InfoLevel}, {msg: fmt.Sprintf("config.request_limits.max_num_values: %d", expectedConfig.RequestLimits.MaxNumValues), lvl: logrus.InfoLevel}, + {msg: fmt.Sprintf("config.request_limits.max_header_size_bytes: %d", expectedConfig.RequestLimits.MaxHeaderSize), lvl: logrus.InfoLevel}, {msg: fmt.Sprintf("config.backend.type: %s", expectedConfig.Backend.Type), lvl: logrus.InfoLevel}, {msg: fmt.Sprintf("config.compression.type: %s", expectedConfig.Compression.Type), lvl: logrus.InfoLevel}, {msg: fmt.Sprintf("Prebid Cache will run without metrics"), lvl: logrus.InfoLevel}, @@ -1244,6 +1245,7 @@ func getExpectedFullConfigForTestFile() Configuration { MaxNumValues: 10, MaxTTLSeconds: 5000, AllowSettingKeys: true, + MaxHeaderSize: 16384, //16KiB }, Backend: Backend{ Type: BackendMemory, diff --git a/config/configtest/sample_full_config.yaml b/config/configtest/sample_full_config.yaml index 3c40fbb7..23854c9f 100644 --- a/config/configtest/sample_full_config.yaml +++ b/config/configtest/sample_full_config.yaml @@ -11,6 +11,7 @@ request_limits: max_num_values: 10 max_ttl_seconds: 5000 allow_setting_keys: true + max_header_size_bytes: 16384 backend: type: "memory" aerospike: diff --git a/server/server.go b/server/server.go index 4f75d5a7..9da84909 100644 --- a/server/server.go +++ b/server/server.go @@ -74,20 +74,36 @@ func Listen(cfg config.Configuration, publicHandler http.Handler, adminHandler h return } +// newAdminServer returns an http.Server with the configured with the AdminPort and +// RequestLimits.MaxHeaderBytes values specified in Prebid Cache's config files or +// environment variables. If RequestLimits.MaxHeaderBytes is zero or non-specified, +// the http library sets server.MaxHeaderBytes to the value of http.DefaultMaxHeaderBytes func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server { - return &http.Server{ + server := &http.Server{ Addr: ":" + strconv.Itoa(cfg.AdminPort), Handler: handler, } + if cfg.RequestLimits.MaxHeaderSize > 0 { + server.MaxHeaderBytes = cfg.RequestLimits.MaxHeaderSize + } + return server } +// newMainServer returns an http.Server with the configured with the Port and +// RequestLimits.MaxHeaderBytes values specified in Prebid Cache's config files +// or environment variables. If RequestLimits.MaxHeaderBytes is zero or non-specified, +// 1 MB, which is the value of the http library's DefaultMaxHeaderBytes, is set instead. func newMainServer(cfg config.Configuration, handler http.Handler) *http.Server { - return &http.Server{ + server := &http.Server{ Addr: ":" + strconv.Itoa(cfg.Port), Handler: handler, ReadTimeout: 15 * time.Second, WriteTimeout: 15 * time.Second, } + if cfg.RequestLimits.MaxHeaderSize > 0 { + server.MaxHeaderBytes = cfg.RequestLimits.MaxHeaderSize + } + return server } func runServer(server *http.Server, name string, listener net.Listener) { From ac6b5fbc5588af38894edb72a171ed0a5ee836dd Mon Sep 17 00:00:00 2001 From: guscarreon Date: Thu, 25 Apr 2024 16:28:18 -0400 Subject: [PATCH 2/5] Alex's review --- config/config_test.go | 12 ++++++++++++ server/server.go | 15 ++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 6ac4e579..c9f35632 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -867,6 +867,18 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { }, expectFatal: true, }, + { + description: "Negative max_num_values, expect fatal level log and early exit", + inRequestLimitsCfg: &RequestLimits{MaxHeaderSize: -1}, + expectedLogInfo: []logComponents{ + {msg: `config.request_limits.allow_setting_keys: false`, lvl: logrus.InfoLevel}, + {msg: `config.request_limits.max_ttl_seconds: 0`, lvl: logrus.InfoLevel}, + {msg: `config.request_limits.max_size_bytes: 0`, lvl: logrus.InfoLevel}, + {msg: `config.request_limits.max_num_values: 0`, lvl: logrus.InfoLevel}, + {msg: `invalid config.request_limits.max_header_size_bytes: -1. Value cannot be negative.`, lvl: logrus.FatalLevel}, + }, + expectFatal: true, + }, } //substitute logger exit function so execution doesn't get interrupted diff --git a/server/server.go b/server/server.go index 9da84909..c847d510 100644 --- a/server/server.go +++ b/server/server.go @@ -74,10 +74,10 @@ func Listen(cfg config.Configuration, publicHandler http.Handler, adminHandler h return } -// newAdminServer returns an http.Server with the configured with the AdminPort and -// RequestLimits.MaxHeaderBytes values specified in Prebid Cache's config files or -// environment variables. If RequestLimits.MaxHeaderBytes is zero or non-specified, -// the http library sets server.MaxHeaderBytes to the value of http.DefaultMaxHeaderBytes +// newAdminServer returns an http.Server with the AdminPort and RequestLimits.MaxHeaderBytes +// from Prebid Cache's config files or environment variables. If RequestLimits.MaxHeaderBytes +// is zero or was not specified the the http library's DefaultMaxHeaderBytes value of 1 MB +// is set instead. func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server { server := &http.Server{ Addr: ":" + strconv.Itoa(cfg.AdminPort), @@ -89,10 +89,11 @@ func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server return server } -// newMainServer returns an http.Server with the configured with the Port and +// newMainServer returns an http.Server with the configured Port and // RequestLimits.MaxHeaderBytes values specified in Prebid Cache's config files -// or environment variables. If RequestLimits.MaxHeaderBytes is zero or non-specified, -// 1 MB, which is the value of the http library's DefaultMaxHeaderBytes, is set instead. +// or environment variables. If RequestLimits.MaxHeaderBytes is zero or was not +// specified, 1 MB, which is the value of the http library's DefaultMaxHeaderBytes, +// is set instead. func newMainServer(cfg config.Configuration, handler http.Handler) *http.Server { server := &http.Server{ Addr: ":" + strconv.Itoa(cfg.Port), From 5cafdc0c616c977de1fb1fc70724a09c9865c5e3 Mon Sep 17 00:00:00 2001 From: guscarreon Date: Mon, 29 Apr 2024 20:25:29 -0400 Subject: [PATCH 3/5] Comment correction --- server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index c847d510..6f103e2c 100644 --- a/server/server.go +++ b/server/server.go @@ -76,7 +76,7 @@ func Listen(cfg config.Configuration, publicHandler http.Handler, adminHandler h // newAdminServer returns an http.Server with the AdminPort and RequestLimits.MaxHeaderBytes // from Prebid Cache's config files or environment variables. If RequestLimits.MaxHeaderBytes -// is zero or was not specified the the http library's DefaultMaxHeaderBytes value of 1 MB +// is zero or was not specified, the http library's DefaultMaxHeaderBytes value of 1 MB // is set instead. func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server { server := &http.Server{ From 0929041d417a6c76d84208ce91712774fdfe0b83 Mon Sep 17 00:00:00 2001 From: guscarreon Date: Thu, 2 May 2024 13:00:24 -0400 Subject: [PATCH 4/5] Cap to http.DefaultMaxHeaderBytes --- config/config.go | 7 ++++--- config/config_test.go | 5 +++-- server/server.go | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/config/config.go b/config/config.go index 2821cc95..ebf926fc 100644 --- a/config/config.go +++ b/config/config.go @@ -1,6 +1,7 @@ package config import ( + "net/http" "strings" "time" @@ -95,7 +96,7 @@ func setConfigDefaults(v *viper.Viper) { v.SetDefault("request_limits.max_size_bytes", utils.REQUEST_MAX_SIZE_BYTES) v.SetDefault("request_limits.max_num_values", utils.REQUEST_MAX_NUM_VALUES) v.SetDefault("request_limits.max_ttl_seconds", utils.REQUEST_MAX_TTL_SECONDS) - v.SetDefault("request_limits.max_header_size_bytes", 0) + v.SetDefault("request_limits.max_header_size_bytes", http.DefaultMaxHeaderBytes) v.SetDefault("routes.allow_public_write", true) } @@ -204,10 +205,10 @@ func (cfg *RequestLimits) validateAndLog() { log.Fatalf("invalid config.request_limits.max_num_values: %d. Value cannot be negative.", cfg.MaxNumValues) } - if cfg.MaxHeaderSize >= 0 { + if cfg.MaxHeaderSize >= 0 && cfg.MaxHeaderSize <= http.DefaultMaxHeaderBytes { log.Infof("config.request_limits.max_header_size_bytes: %d", cfg.MaxHeaderSize) } else { - log.Fatalf("invalid config.request_limits.max_header_size_bytes: %d. Value cannot be negative.", cfg.MaxHeaderSize) + log.Fatalf("invalid config.request_limits.max_header_size_bytes: %d. Value out of range.", cfg.MaxHeaderSize) } } diff --git a/config/config_test.go b/config/config_test.go index c9f35632..b4ff9b70 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -868,14 +868,14 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { expectFatal: true, }, { - description: "Negative max_num_values, expect fatal level log and early exit", + description: "Negative max_header_size_bytes, expect fatal level log and early exit", inRequestLimitsCfg: &RequestLimits{MaxHeaderSize: -1}, expectedLogInfo: []logComponents{ {msg: `config.request_limits.allow_setting_keys: false`, lvl: logrus.InfoLevel}, {msg: `config.request_limits.max_ttl_seconds: 0`, lvl: logrus.InfoLevel}, {msg: `config.request_limits.max_size_bytes: 0`, lvl: logrus.InfoLevel}, {msg: `config.request_limits.max_num_values: 0`, lvl: logrus.InfoLevel}, - {msg: `invalid config.request_limits.max_header_size_bytes: -1. Value cannot be negative.`, lvl: logrus.FatalLevel}, + {msg: `invalid config.request_limits.max_header_size_bytes: -1. Value out of range.`, lvl: logrus.FatalLevel}, }, expectFatal: true, }, @@ -1232,6 +1232,7 @@ func getExpectedDefaultConfig() Configuration { MaxSize: 10240, MaxNumValues: 10, MaxTTLSeconds: 3600, + MaxHeaderSize: 1048576, }, Routes: Routes{ AllowPublicWrite: true, diff --git a/server/server.go b/server/server.go index 6f103e2c..1d68d18b 100644 --- a/server/server.go +++ b/server/server.go @@ -83,7 +83,7 @@ func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server Addr: ":" + strconv.Itoa(cfg.AdminPort), Handler: handler, } - if cfg.RequestLimits.MaxHeaderSize > 0 { + if cfg.RequestLimits.MaxHeaderSize > 0 && cfg.RequestLimits.MaxHeaderSize < http.DefaultMaxHeaderBytes { server.MaxHeaderBytes = cfg.RequestLimits.MaxHeaderSize } return server From b21491684ad1c82040f199b65ebbca865a6cf830 Mon Sep 17 00:00:00 2001 From: guscarreon Date: Thu, 2 May 2024 14:46:17 -0400 Subject: [PATCH 5/5] Rolled back max header size cap --- config/config.go | 4 ++-- config/config_test.go | 2 +- server/server.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index ebf926fc..18f41dd5 100644 --- a/config/config.go +++ b/config/config.go @@ -205,10 +205,10 @@ func (cfg *RequestLimits) validateAndLog() { log.Fatalf("invalid config.request_limits.max_num_values: %d. Value cannot be negative.", cfg.MaxNumValues) } - if cfg.MaxHeaderSize >= 0 && cfg.MaxHeaderSize <= http.DefaultMaxHeaderBytes { + if cfg.MaxHeaderSize >= 0 { log.Infof("config.request_limits.max_header_size_bytes: %d", cfg.MaxHeaderSize) } else { - log.Fatalf("invalid config.request_limits.max_header_size_bytes: %d. Value out of range.", cfg.MaxHeaderSize) + log.Fatalf("invalid config.request_limits.max_header_size_bytes: %d. Value cannot be negative.", cfg.MaxHeaderSize) } } diff --git a/config/config_test.go b/config/config_test.go index b4ff9b70..1c12cdb1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -875,7 +875,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { {msg: `config.request_limits.max_ttl_seconds: 0`, lvl: logrus.InfoLevel}, {msg: `config.request_limits.max_size_bytes: 0`, lvl: logrus.InfoLevel}, {msg: `config.request_limits.max_num_values: 0`, lvl: logrus.InfoLevel}, - {msg: `invalid config.request_limits.max_header_size_bytes: -1. Value out of range.`, lvl: logrus.FatalLevel}, + {msg: `invalid config.request_limits.max_header_size_bytes: -1. Value cannot be negative.`, lvl: logrus.FatalLevel}, }, expectFatal: true, }, diff --git a/server/server.go b/server/server.go index 1d68d18b..6f103e2c 100644 --- a/server/server.go +++ b/server/server.go @@ -83,7 +83,7 @@ func newAdminServer(cfg config.Configuration, handler http.Handler) *http.Server Addr: ":" + strconv.Itoa(cfg.AdminPort), Handler: handler, } - if cfg.RequestLimits.MaxHeaderSize > 0 && cfg.RequestLimits.MaxHeaderSize < http.DefaultMaxHeaderBytes { + if cfg.RequestLimits.MaxHeaderSize > 0 { server.MaxHeaderBytes = cfg.RequestLimits.MaxHeaderSize } return server