diff --git a/adapters/audienceNetwork/facebook.go b/adapters/audienceNetwork/facebook.go index 2a59e3c1699..dc4042412da 100644 --- a/adapters/audienceNetwork/facebook.go +++ b/adapters/audienceNetwork/facebook.go @@ -433,7 +433,7 @@ func Builder(bidderName openrtb_ext.BidderName, config config.Adapter, server co return bidder, nil } -func (a *adapter) MakeTimeoutNotification(req *adapters.RequestData) (*adapters.RequestData, []error) { +func (a *adapter) MakeTimeoutNotification(req *adapters.RequestData) (*adapters.RequestData, error) { var ( rID string pubID string @@ -445,15 +445,13 @@ func (a *adapter) MakeTimeoutNotification(req *adapters.RequestData) (*adapters. // corresponding imp's ID rID, err = jsonparser.GetString(req.Body, "id") if err != nil { - return &adapters.RequestData{}, []error{err} + return &adapters.RequestData{}, err } // The publisher ID is expected in the app object pubID, err = jsonparser.GetString(req.Body, "app", "publisher", "id") if err != nil { - return &adapters.RequestData{}, []error{ - errors.New("path app.publisher.id not found in the request"), - } + return &adapters.RequestData{}, errors.New("path app.publisher.id not found in the request") } uri := fmt.Sprintf("https://www.facebook.com/audiencenetwork/nurl/?partner=%s&app=%s&auction=%s&ortb_loss_code=2", a.platformID, pubID, rID) diff --git a/adapters/bidder.go b/adapters/bidder.go index e4862cb2df5..fb0ec07e032 100644 --- a/adapters/bidder.go +++ b/adapters/bidder.go @@ -51,7 +51,7 @@ type TimeoutBidder interface { // // Do note that if MakeRequests returns multiple requests, and more than one of these times out, MakeTimeoutNotice will be called // once for each timed out request. - MakeTimeoutNotification(req *RequestData) (*RequestData, []error) + MakeTimeoutNotification(req *RequestData) (*RequestData, error) } // BidderResponse wraps the server's response with the list of bids and the currency used by the bidder. diff --git a/analytics/agma/sender.go b/analytics/agma/sender.go index b47b059536e..7ae2849a82a 100644 --- a/analytics/agma/sender.go +++ b/analytics/agma/sender.go @@ -5,6 +5,7 @@ import ( "compress/gzip" "context" "fmt" + "io" "net/http" "net/url" "time" @@ -74,6 +75,12 @@ func createHttpSender(httpClient *http.Client, endpoint config.AgmaAnalyticsHttp glog.Errorf("[agmaAnalytics] Sending request failed %v", err) return err } + defer func() { + if _, err := io.Copy(io.Discard, resp.Body); err != nil { + glog.Errorf("[agmaAnalytics] Draining response body failed: %v", err) + } + resp.Body.Close() + }() if resp.StatusCode != http.StatusOK { glog.Errorf("[agmaAnalytics] Wrong code received %d instead of %d", resp.StatusCode, http.StatusOK) diff --git a/analytics/pubstack/config.go b/analytics/pubstack/config.go index 3f604af58e9..28330fc362c 100644 --- a/analytics/pubstack/config.go +++ b/analytics/pubstack/config.go @@ -2,11 +2,14 @@ package pubstack import ( "encoding/json" + "fmt" + "io" "net/http" "net/url" "time" "github.com/docker/go-units" + "github.com/golang/glog" ) func fetchConfig(client *http.Client, endpoint *url.URL) (*Configuration, error) { @@ -14,12 +17,23 @@ func fetchConfig(client *http.Client, endpoint *url.URL) (*Configuration, error) if err != nil { return nil, err } + defer func() { + // read the entire response body to ensure full connection reuse if there's an + // error while decoding the json + if _, err := io.Copy(io.Discard, res.Body); err != nil { + glog.Errorf("[pubstack] Draining config response body failed: %v", err) + } + res.Body.Close() + }() + + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("unexpected status code: %d", res.StatusCode) + } - defer res.Body.Close() c := Configuration{} err = json.NewDecoder(res.Body).Decode(&c) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to decode config: %w", err) } return &c, nil } diff --git a/analytics/pubstack/eventchannel/sender.go b/analytics/pubstack/eventchannel/sender.go index fe068b1555f..c85c4fe446f 100644 --- a/analytics/pubstack/eventchannel/sender.go +++ b/analytics/pubstack/eventchannel/sender.go @@ -3,6 +3,7 @@ package eventchannel import ( "bytes" "fmt" + "io" "net/http" "net/url" "path" @@ -27,7 +28,12 @@ func NewHttpSender(client *http.Client, endpoint string) Sender { if err != nil { return err } - resp.Body.Close() + defer func() { + if _, err := io.Copy(io.Discard, resp.Body); err != nil { + glog.Errorf("[pubstack] Draining sender response body failed: %v", err) + } + resp.Body.Close() + }() if resp.StatusCode != http.StatusOK { glog.Errorf("[pubstack] Wrong code received %d instead of %d", resp.StatusCode, http.StatusOK) diff --git a/config/config.go b/config/config.go index 2716f99077e..de6d04b9307 100644 --- a/config/config.go +++ b/config/config.go @@ -472,15 +472,19 @@ type Analytics struct { } type CurrencyConverter struct { - FetchURL string `mapstructure:"fetch_url"` - FetchIntervalSeconds int `mapstructure:"fetch_interval_seconds"` - StaleRatesSeconds int `mapstructure:"stale_rates_seconds"` + FetchURL string `mapstructure:"fetch_url"` + FetchTimeoutMilliseconds int `mapstructure:"fetch_timeout_ms"` + FetchIntervalSeconds int `mapstructure:"fetch_interval_seconds"` + StaleRatesSeconds int `mapstructure:"stale_rates_seconds"` } func (cfg *CurrencyConverter) validate(errs []error) []error { if cfg.FetchIntervalSeconds < 0 { errs = append(errs, fmt.Errorf("currency_converter.fetch_interval_seconds must be in the range [0, %d]. Got %d", 0xffff, cfg.FetchIntervalSeconds)) } + if cfg.FetchTimeoutMilliseconds < 0 { + errs = append(errs, fmt.Errorf("currency_converter.fetch_timeout_ms must be 0 or greater. Got %d", cfg.FetchTimeoutMilliseconds)) + } return errs } @@ -1173,6 +1177,7 @@ func SetupViper(v *viper.Viper, filename string, bidderInfos BidderInfos) { v.SetDefault("ccpa.enforce", false) v.SetDefault("lmt.enforce", true) v.SetDefault("currency_converter.fetch_url", "https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json") + v.SetDefault("currency_converter.fetch_timeout_ms", 60000) // 60 seconds v.SetDefault("currency_converter.fetch_interval_seconds", 1800) // fetch currency rates every 30 minutes v.SetDefault("currency_converter.stale_rates_seconds", 0) v.SetDefault("default_request.type", "") diff --git a/currency/currency_test.go b/currency/currency_test.go index e86ee4537fe..c29c8d18e79 100644 --- a/currency/currency_test.go +++ b/currency/currency_test.go @@ -53,6 +53,7 @@ func TestGetAuctionCurrencyRates(t *testing.T) { return NewRateConverter( mockCurrencyClient, + 60*time.Second, "currency.fake.com", 24*time.Hour, ) diff --git a/currency/rate_converter.go b/currency/rate_converter.go index 2e39b57f403..d1898a59a22 100644 --- a/currency/rate_converter.go +++ b/currency/rate_converter.go @@ -1,6 +1,7 @@ package currency import ( + "context" "fmt" "io" "net/http" @@ -16,6 +17,7 @@ import ( // RateConverter holds the currencies conversion rates dictionary type RateConverter struct { httpClient httpClient + httpTimeout time.Duration staleRatesThreshold time.Duration syncSourceURL string rates atomic.Value // Should only hold Rates struct @@ -27,11 +29,13 @@ type RateConverter struct { // NewRateConverter returns a new RateConverter func NewRateConverter( httpClient httpClient, + httpTimeout time.Duration, syncSourceURL string, staleRatesThreshold time.Duration, ) *RateConverter { return &RateConverter{ httpClient: httpClient, + httpTimeout: httpTimeout, staleRatesThreshold: staleRatesThreshold, syncSourceURL: syncSourceURL, rates: atomic.Value{}, @@ -43,7 +47,10 @@ func NewRateConverter( // fetch allows to retrieve the currencies rates from the syncSourceURL provided func (rc *RateConverter) fetch() (*Rates, error) { - request, err := http.NewRequest("GET", rc.syncSourceURL, nil) + ctx, cancel := context.WithTimeout(context.Background(), rc.httpTimeout) + defer cancel() + + request, err := http.NewRequestWithContext(ctx, "GET", rc.syncSourceURL, nil) if err != nil { return nil, err } @@ -52,23 +59,29 @@ func (rc *RateConverter) fetch() (*Rates, error) { if err != nil { return nil, err } + defer func() { + // read the entire response body to ensure full connection reuse if there's an + // invalid status code + if _, err := io.Copy(io.Discard, response.Body); err != nil { + glog.Errorf("error draining conversion rates response body: %v", err) + } + response.Body.Close() + }() if response.StatusCode >= 400 { - message := fmt.Sprintf("The currency rates request failed with status code %d", response.StatusCode) + message := fmt.Sprintf("the currency rates request failed with status code %d", response.StatusCode) return nil, &errortypes.BadServerResponse{Message: message} } - defer response.Body.Close() - bytesJSON, err := io.ReadAll(response.Body) if err != nil { - return nil, err + return nil, fmt.Errorf("the currency rates request failed: %v", err) } updatedRates := &Rates{} err = jsonutil.UnmarshalValid(bytesJSON, updatedRates) if err != nil { - return nil, err + return nil, fmt.Errorf("the currency rates request failed to parse json: %v", err) } return updatedRates, err diff --git a/currency/rate_converter_test.go b/currency/rate_converter_test.go index 95049916194..0698a7ba270 100644 --- a/currency/rate_converter_test.go +++ b/currency/rate_converter_test.go @@ -123,6 +123,7 @@ func TestReadWriteRates(t *testing.T) { } currencyConverter := NewRateConverter( &http.Client{}, + 60*time.Second, url, 24*time.Hour, ) @@ -181,6 +182,7 @@ func TestRateStaleness(t *testing.T) { // Execute: currencyConverter := NewRateConverter( &http.Client{}, + 60*time.Second, mockedHttpServer.URL, 30*time.Second, // stale rates threshold ) @@ -268,6 +270,7 @@ func TestRatesAreNeverConsideredStale(t *testing.T) { // Execute: currencyConverter := NewRateConverter( &http.Client{}, + 60*time.Second, mockedHttpServer.URL, 0*time.Millisecond, // stale rates threshold ) @@ -321,6 +324,7 @@ func TestRace(t *testing.T) { interval := 1 * time.Millisecond currencyConverter := NewRateConverter( mockedHttpClient, + 60*time.Second, "currency.fake.com", 24*time.Hour, ) diff --git a/endpoints/openrtb2/auction_benchmark_test.go b/endpoints/openrtb2/auction_benchmark_test.go index 5dfaecdda91..ab8cb4863ac 100644 --- a/endpoints/openrtb2/auction_benchmark_test.go +++ b/endpoints/openrtb2/auction_benchmark_test.go @@ -94,7 +94,7 @@ func BenchmarkOpenrtbEndpoint(b *testing.B) { nilMetrics, infos, gdprPermsBuilder, - currency.NewRateConverter(&http.Client{}, "", time.Duration(0)), + currency.NewRateConverter(&http.Client{}, 60*time.Second, "", time.Duration(0)), empty_fetcher.EmptyFetcher{}, &adscert.NilSigner{}, macros.NewStringIndexBasedReplacer(), diff --git a/endpoints/openrtb2/test_utils.go b/endpoints/openrtb2/test_utils.go index b19cb5badab..75528c5f9dc 100644 --- a/endpoints/openrtb2/test_utils.go +++ b/endpoints/openrtb2/test_utils.go @@ -1255,7 +1255,7 @@ func buildTestExchange(testCfg *testConfigValues, adapterMap map[openrtb_ext.Bid } } - mockCurrencyConverter := currency.NewRateConverter(mockCurrencyRatesServer.Client(), mockCurrencyRatesServer.URL, time.Second) + mockCurrencyConverter := currency.NewRateConverter(mockCurrencyRatesServer.Client(), time.Second, mockCurrencyRatesServer.URL, time.Second) mockCurrencyConverter.Run() gdprPermsBuilder := fakePermissionsBuilder{ diff --git a/exchange/bidder.go b/exchange/bidder.go index cd3bb369197..a305a0e84b4 100644 --- a/exchange/bidder.go +++ b/exchange/bidder.go @@ -636,6 +636,7 @@ func (bidder *BidderAdapter) doRequestImpl(ctx context.Context, req *adapters.Re err: err, } } + defer httpResp.Body.Close() respBody, err := io.ReadAll(httpResp.Body) if err != nil { @@ -644,7 +645,6 @@ func (bidder *BidderAdapter) doRequestImpl(ctx context.Context, req *adapters.Re err: err, } } - defer httpResp.Body.Close() if httpResp.StatusCode < 200 || httpResp.StatusCode >= 400 { if httpResp.StatusCode >= 500 { @@ -671,14 +671,25 @@ func (bidder *BidderAdapter) doRequestImpl(ctx context.Context, req *adapters.Re func (bidder *BidderAdapter) doTimeoutNotification(timeoutBidder adapters.TimeoutBidder, req *adapters.RequestData, logger util.LogMsg) { ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() - toReq, errL := timeoutBidder.MakeTimeoutNotification(req) - if toReq != nil && len(errL) == 0 { + + toReq, errMakeTimeoutNotification := timeoutBidder.MakeTimeoutNotification(req) + if toReq != nil && errMakeTimeoutNotification == nil { httpReq, err := http.NewRequest(toReq.Method, toReq.Uri, bytes.NewBuffer(toReq.Body)) if err == nil { httpReq.Header = req.Headers httpResp, err := ctxhttp.Do(ctx, bidder.Client, httpReq) + if err == nil { + defer func() { + if _, err := io.Copy(io.Discard, httpResp.Body); err != nil { + glog.Errorf("TimeoutNotification: Draining response body failed %v", err) + } + httpResp.Body.Close() + }() + } + success := (err == nil && httpResp.StatusCode >= 200 && httpResp.StatusCode < 300) bidder.me.RecordTimeoutNotice(success) + if bidder.config.Debug.TimeoutNotification.Log && !(bidder.config.Debug.TimeoutNotification.FailOnly && success) { var msg string if err == nil { @@ -697,16 +708,15 @@ func (bidder *BidderAdapter) doTimeoutNotification(timeoutBidder adapters.Timeou } } } else if bidder.config.Debug.TimeoutNotification.Log { - reqJSON, err := jsonutil.Marshal(req) + reqJSON, errMarshal := jsonutil.Marshal(req) var msg string - if err == nil { - msg = fmt.Sprintf("TimeoutNotification: Failed to generate timeout request: error(%s), bidder request(%s)", errL[0].Error(), string(reqJSON)) + if errMarshal == nil { + msg = fmt.Sprintf("TimeoutNotification: Failed to generate timeout request: error(%s), bidder request(%s)", errMarshal.Error(), string(reqJSON)) } else { - msg = fmt.Sprintf("TimeoutNotification: Failed to generate timeout request: error(%s), bidder request marshal failed(%s)", errL[0].Error(), err.Error()) + msg = fmt.Sprintf("TimeoutNotification: Failed to generate timeout request: error(%s), bidder request marshal failed(%s)", errMakeTimeoutNotification.Error(), errMarshal.Error()) } util.LogRandomSample(msg, logger, bidder.config.Debug.TimeoutNotification.SamplingRate) } - } type httpCallInfo struct { diff --git a/exchange/bidder_test.go b/exchange/bidder_test.go index dbb167e053d..50e919c2a6c 100644 --- a/exchange/bidder_test.go +++ b/exchange/bidder_test.go @@ -104,7 +104,7 @@ func TestSingleBidder(t *testing.T) { bidderImpl.bidResponse = mockBidderResponse bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, test.debugInfo, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -229,7 +229,7 @@ func TestSingleBidderGzip(t *testing.T) { bidderImpl.bidResponse = mockBidderResponse bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, test.debugInfo, "GZIP") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -330,7 +330,7 @@ func TestRequestBidRemovesSensitiveHeaders(t *testing.T) { ctx := context.Background() bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, debugInfo, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -384,7 +384,7 @@ func TestSetGPCHeader(t *testing.T) { ctx := context.Background() bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, debugInfo, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, BidderName: "test", @@ -435,7 +435,7 @@ func TestSetGPCHeaderNil(t *testing.T) { ctx := context.Background() bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, debugInfo, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -508,7 +508,7 @@ func TestMultiBidder(t *testing.T) { bidResponse: mockBidderResponse, } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, BidderName: "test", @@ -877,6 +877,7 @@ func TestMultiCurrencies(t *testing.T) { bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") currencyConverter := currency.NewRateConverter( &http.Client{}, + 60*time.Second, mockedHTTPServer.URL, time.Duration(24)*time.Hour, ) @@ -1043,7 +1044,7 @@ func TestMultiCurrencies_RateConverterNotSet(t *testing.T) { // Execute: bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, BidderName: "test", @@ -1221,6 +1222,7 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") currencyConverter := currency.NewRateConverter( &http.Client{}, + 60*time.Second, mockedHTTPServer.URL, time.Duration(24)*time.Hour, ) @@ -1541,7 +1543,7 @@ func TestMobileNativeTypes(t *testing.T) { bidResponse: tc.mockBidderResponse, } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: tc.mockBidderRequest, @@ -1699,7 +1701,7 @@ func TestRequestBidsStoredBidResponses(t *testing.T) { bidderImpl := &goodSingleBidderWithStoredBidResp{} bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: tc.mockBidderRequest, @@ -1805,7 +1807,7 @@ func TestFledge(t *testing.T) { bidResponses: tc.mockBidderResponse, } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderOpenx, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{ @@ -1851,7 +1853,7 @@ func TestFledge(t *testing.T) { func TestErrorReporting(t *testing.T) { bidder := AdaptBidder(&bidRejector{}, nil, &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, BidderName: "test", @@ -2086,7 +2088,7 @@ func TestCallRecordAdapterConnections(t *testing.T) { // Run requestBid using an http.Client with a mock handler bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, mockMetricEngine, openrtb_ext.BidderAppnexus, nil, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2333,7 +2335,7 @@ func TestRequestBidsWithAdsCertsSigner(t *testing.T) { } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{Allow: false}, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2473,7 +2475,7 @@ func (bidder *notifyingBidder) MakeBids(internalRequest *openrtb2.BidRequest, ex return nil, nil } -func (bidder *notifyingBidder) MakeTimeoutNotification(req *adapters.RequestData) (*adapters.RequestData, []error) { +func (bidder *notifyingBidder) MakeTimeoutNotification(req *adapters.RequestData) (*adapters.RequestData, error) { return &bidder.notifyRequest, nil } @@ -2541,7 +2543,7 @@ func TestExtraBid(t *testing.T) { } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2655,7 +2657,7 @@ func TestExtraBidWithAlternateBidderCodeDisabled(t *testing.T) { } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2761,7 +2763,7 @@ func TestExtraBidWithBidAdjustments(t *testing.T) { } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2874,7 +2876,7 @@ func TestExtraBidWithBidAdjustmentsUsingAdapterCode(t *testing.T) { } bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, "") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) bidderReq := BidderRequest{ BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "impId"}}}, @@ -2996,6 +2998,7 @@ func TestExtraBidWithMultiCurrencies(t *testing.T) { bidder := AdaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.NilMetricsEngine{}, openrtb_ext.BidderAppnexus, nil, "") currencyConverter := currency.NewRateConverter( &http.Client{}, + 60*time.Second, mockedHTTPServer.URL, time.Duration(24)*time.Hour, ) @@ -3267,7 +3270,7 @@ func TestUpdateBidderTmax(t *testing.T) { requestHeaders := http.Header{} requestHeaders.Add("Content-Type", "application/json") - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) var requestTmax int64 = 700 bidderReq := BidderRequest{ diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index 2e8fcbaa9df..c4087b4d681 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -76,7 +76,7 @@ func TestNewExchange(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) gdprPermsBuilder := fakePermissionsBuilder{ permissions: &permissionsMock{ @@ -126,7 +126,7 @@ func TestCharacterEscape(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) gdprPermsBuilder := fakePermissionsBuilder{ permissions: &permissionsMock{ @@ -333,7 +333,7 @@ func TestDebugBehaviour(t *testing.T) { allowAllBidders: true, }, }.Builder - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.categoriesFetcher = categoriesFetcher e.requestSplitter = requestSplitter{ me: &metricsConf.NilMetricsEngine{}, @@ -499,7 +499,7 @@ func TestTwoBiddersDebugDisabledAndEnabled(t *testing.T) { allowAllBidders: true, }, }.Builder - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.categoriesFetcher = categoriesFetcher e.requestSplitter = requestSplitter{ me: e.me, @@ -581,6 +581,7 @@ func TestOverrideWithCustomCurrency(t *testing.T) { } mockCurrencyConverter := currency.NewRateConverter( mockCurrencyClient, + 60*time.Second, "currency.fake.com", 24*time.Hour, ) @@ -748,6 +749,7 @@ func TestAdapterCurrency(t *testing.T) { } currencyConverter := currency.NewRateConverter( mockCurrencyClient, + 60*time.Second, "currency.fake.com", 24*time.Hour, ) @@ -832,6 +834,7 @@ func TestFloorsSignalling(t *testing.T) { } currencyConverter := currency.NewRateConverter( mockCurrencyClient, + 60*time.Second, "currency.com", 24*time.Hour, ) @@ -1130,7 +1133,7 @@ func TestReturnCreativeEndToEnd(t *testing.T) { allowAllBidders: true, }, }.Builder - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.categoriesFetcher = categoriesFetcher e.bidIDGenerator = &fakeBidIDGenerator{GenerateBidID: false, ReturnError: false} e.requestSplitter = requestSplitter{ @@ -1228,7 +1231,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) { if adaptersErr != nil { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) pbc := pbc.NewClient(&http.Client{}, &cfg.CacheURL, &cfg.ExtCacheURL, testEngine) gdprPermsBuilder := fakePermissionsBuilder{ @@ -1429,7 +1432,7 @@ func TestBidReturnsCreative(t *testing.T) { e.cache = &wellBehavedCache{} e.me = &metricsConf.NilMetricsEngine{} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) //Run tests for _, test := range testCases { @@ -1588,7 +1591,7 @@ func TestBidResponseCurrency(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) gdprPermsBuilder := fakePermissionsBuilder{ permissions: &permissionsMock{ @@ -1820,7 +1823,7 @@ func TestRaceIntegration(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) auctionRequest := &AuctionRequest{ BidRequestWrapper: &openrtb_ext.RequestWrapper{BidRequest: getTestBuildRequest(t)}, @@ -1928,7 +1931,7 @@ func TestPanicRecovery(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) gdprPermsBuilder := fakePermissionsBuilder{ permissions: &permissionsMock{ @@ -1994,7 +1997,7 @@ func TestPanicRecoveryHighLevel(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) categoriesFetcher, error := newCategoryFetcher("./test/category-mapping") if error != nil { @@ -2466,7 +2469,7 @@ func newExchangeForTests(t *testing.T, filename string, aliases map[string]strin me: metricsEngine, cache: &wellBehavedCache{}, cacheTime: 0, - currencyConverter: currency.NewRateConverter(&http.Client{}, "", time.Duration(0)), + currencyConverter: currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)), gdprDefaultValue: gdprDefaultValue, gdprPermsBuilder: gdprPermsBuilder, privacyConfig: privacyConfig, @@ -4142,7 +4145,7 @@ func TestStoredAuctionResponses(t *testing.T) { e.me = &metricsConf.NilMetricsEngine{} e.categoriesFetcher = categoriesFetcher e.bidIDGenerator = &fakeBidIDGenerator{GenerateBidID: false, ReturnError: false} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.gdprPermsBuilder = fakePermissionsBuilder{ permissions: &permissionsMock{ allowAllBidders: true, @@ -4508,7 +4511,7 @@ func TestAuctionDebugEnabled(t *testing.T) { e.me = &metricsConf.NilMetricsEngine{} e.categoriesFetcher = categoriesFetcher e.bidIDGenerator = &fakeBidIDGenerator{GenerateBidID: false, ReturnError: false} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.gdprPermsBuilder = fakePermissionsBuilder{ permissions: &permissionsMock{ allowAllBidders: true, @@ -4576,7 +4579,7 @@ func TestPassExperimentConfigsToHoldAuction(t *testing.T) { t.Fatalf("Error intializing adapters: %v", adaptersErr) } - currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + currencyConverter := currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) gdprPermsBuilder := fakePermissionsBuilder{ permissions: &permissionsMock{ @@ -4909,7 +4912,7 @@ func TestMakeBidWithValidation(t *testing.T) { e.cache = &wellBehavedCache{} e.me = &metricsConf.NilMetricsEngine{} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) ImpExtInfoMap := make(map[string]ImpExtInfo) ImpExtInfoMap["1"] = ImpExtInfo{} @@ -5105,7 +5108,7 @@ func TestOverrideConfigAlternateBidderCodesWithRequestValues(t *testing.T) { allowAllBidders: true, }, }.Builder - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.categoriesFetcher = categoriesFetcher e.bidIDGenerator = &fakeBidIDGenerator{GenerateBidID: false, ReturnError: false} e.requestSplitter = requestSplitter{ @@ -5890,7 +5893,7 @@ func TestModulesCanBeExecutedForMultipleBiddersSimultaneously(t *testing.T) { e := new(exchange) e.me = &metricsConf.NilMetricsEngine{} - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.requestSplitter = requestSplitter{ me: e.me, gdprPermsBuilder: e.gdprPermsBuilder, @@ -6637,7 +6640,7 @@ func TestAmpEnv(t *testing.T) { allowAllBidders: true, }, }.Builder - e.currencyConverter = currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + e.currencyConverter = currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)) e.categoriesFetcher = categoriesFetcher e.requestSplitter = requestSplitter{ me: &metricsConf.NilMetricsEngine{}, diff --git a/exchange/targeting_test.go b/exchange/targeting_test.go index db356560823..8cbfeca4c5d 100644 --- a/exchange/targeting_test.go +++ b/exchange/targeting_test.go @@ -144,7 +144,7 @@ func runTargetingAuction(t *testing.T, mockBids map[openrtb_ext.BidderName][]*op cache: &wellBehavedCache{}, cacheTime: time.Duration(0), gdprPermsBuilder: gdprPermsBuilder, - currencyConverter: currency.NewRateConverter(&http.Client{}, "", time.Duration(0)), + currencyConverter: currency.NewRateConverter(&http.Client{}, time.Duration(1), "", time.Duration(0)), gdprDefaultValue: gdpr.SignalYes, categoriesFetcher: categoriesFetcher, bidIDGenerator: &fakeBidIDGenerator{GenerateBidID: false, ReturnError: false}, diff --git a/floors/fetcher.go b/floors/fetcher.go index 9fa6ad4f380..c48b45fd7aa 100644 --- a/floors/fetcher.go +++ b/floors/fetcher.go @@ -238,13 +238,13 @@ func (f *PriceFloorFetcher) fetchAndValidate(config config.AccountFloorFetch) (* } if len(floorResp) > (config.MaxFileSizeKB * 1024) { - glog.Errorf("Recieved invalid floor data from URL: %s, reason : floor file size is greater than MaxFileSize", config.URL) + glog.Errorf("Received invalid floor data from URL: %s, reason : floor file size is greater than MaxFileSize", config.URL) return nil, 0 } var priceFloors openrtb_ext.PriceFloorRules if err = json.Unmarshal(floorResp, &priceFloors.Data); err != nil { - glog.Errorf("Recieved invalid price floor json from URL: %s", config.URL) + glog.Errorf("Received invalid price floor json from URL: %s", config.URL) return nil, 0 } @@ -271,6 +271,14 @@ func (f *PriceFloorFetcher) fetchFloorRulesFromURL(config config.AccountFloorFet if err != nil { return nil, 0, errors.New("error while getting response from url : " + err.Error()) } + defer func() { + // read the entire response body to ensure full connection reuse if there's an + // invalid status code + if _, err := io.Copy(io.Discard, httpResp.Body); err != nil { + glog.Errorf("error while draining fetched floor response body: %v", err) + } + httpResp.Body.Close() + }() if httpResp.StatusCode != http.StatusOK { return nil, 0, errors.New("no response from server") @@ -291,7 +299,6 @@ func (f *PriceFloorFetcher) fetchFloorRulesFromURL(config config.AccountFloorFet if err != nil { return nil, 0, errors.New("unable to read response") } - defer httpResp.Body.Close() return respBody, maxAge, nil } diff --git a/main.go b/main.go index 0063b4ee0b6..5e0f5cfd33b 100644 --- a/main.go +++ b/main.go @@ -65,9 +65,10 @@ func loadConfig(bidderInfos config.BidderInfos) (*config.Configuration, error) { } func serve(cfg *config.Configuration) error { + httpTimeout := time.Duration(cfg.CurrencyConverter.FetchTimeoutMilliseconds) * time.Millisecond fetchingInterval := time.Duration(cfg.CurrencyConverter.FetchIntervalSeconds) * time.Second staleRatesThreshold := time.Duration(cfg.CurrencyConverter.StaleRatesSeconds) * time.Second - currencyConverter := currency.NewRateConverter(&http.Client{}, cfg.CurrencyConverter.FetchURL, staleRatesThreshold) + currencyConverter := currency.NewRateConverter(&http.Client{}, httpTimeout, cfg.CurrencyConverter.FetchURL, staleRatesThreshold) currencyConverterTickerTask := task.NewTickerTask(fetchingInterval, currencyConverter) currencyConverterTickerTask.Start() diff --git a/pbs/usersync.go b/pbs/usersync.go index b8c0572c4ac..bde61a7cbdc 100644 --- a/pbs/usersync.go +++ b/pbs/usersync.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "encoding/json" "fmt" + "io" "net/http" "net/url" "strings" @@ -45,7 +46,15 @@ func (deps *UserSyncDeps) VerifyRecaptcha(response string) error { if err != nil { return err } - defer resp.Body.Close() + defer func() { + // read the entire response body to ensure full connection reuse if there's an + // error while decoding the json + if _, err := io.Copy(io.Discard, resp.Body); err != nil { + glog.Errorf("Captcha verify draining response body failed: %v", err) + } + resp.Body.Close() + }() + var gr = googleResponse{} if err := json.NewDecoder(resp.Body).Decode(&gr); err != nil { return err diff --git a/stored_requests/backends/http_fetcher/fetcher.go b/stored_requests/backends/http_fetcher/fetcher.go index a261fb6bdff..5dff2fddba9 100644 --- a/stored_requests/backends/http_fetcher/fetcher.go +++ b/stored_requests/backends/http_fetcher/fetcher.go @@ -92,6 +92,7 @@ func (fetcher *HttpFetcher) FetchRequests(ctx context.Context, requestIDs []stri return nil, nil, []error{err} } defer httpResp.Body.Close() + requestData, impData, errs = unpackResponse(httpResp) return } @@ -142,6 +143,7 @@ func (fetcher *HttpFetcher) FetchAccounts(ctx context.Context, accountIDs []stri } } defer httpResp.Body.Close() + respBytes, err := io.ReadAll(httpResp.Body) if err != nil { return nil, []error{ diff --git a/stored_requests/events/http/http.go b/stored_requests/events/http/http.go index 6ea41a6f848..ec17bd07738 100644 --- a/stored_requests/events/http/http.go +++ b/stored_requests/events/http/http.go @@ -96,6 +96,7 @@ type HTTPEvents struct { func (e *HTTPEvents) fetchAll() { ctx, cancel := e.ctxProducer() defer cancel() + resp, err := ctxhttp.Get(ctx, e.client, e.Endpoint) if respObj, ok := e.parse(e.Endpoint, resp, err); ok && (len(respObj.StoredRequests) > 0 || len(respObj.StoredImps) > 0 || len(respObj.StoredResponses) > 0 || len(respObj.Accounts) > 0) {