diff --git a/exchange/bidder.go b/exchange/bidder.go index d6b8028c2e9..5b6da499093 100644 --- a/exchange/bidder.go +++ b/exchange/bidder.go @@ -167,6 +167,10 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, request *openrtb2.B if debugInfo := ctx.Value(DebugContextKey); debugInfo != nil && debugInfo.(bool) { if accountDebugAllowed { if bidder.config.DebugInfo.Allow { + // it's safe to mutate the request headers since from this point on the + // information is only used for debugging. + removeSensitiveHeaders(httpInfo.request.Headers) + seatBid.httpCalls = append(seatBid.httpCalls, makeExt(httpInfo)) } else { debugDisabledWarning := errortypes.Warning{ @@ -325,25 +329,29 @@ func getAssetByID(id int64, assets []nativeRequests.Asset) (nativeRequests.Asset return nativeRequests.Asset{}, fmt.Errorf("Unable to find asset with ID:%d in the request", id) } +var authorizationHeader = http.CanonicalHeaderKey("authorization") + +// removeSensitiveHeaders mutates the http header object to remove sensitive information. +func removeSensitiveHeaders(h http.Header) { + h.Del(authorizationHeader) +} + // makeExt transforms information about the HTTP call into the contract class for the PBS response. func makeExt(httpInfo *httpCallInfo) *openrtb_ext.ExtHttpCall { - if httpInfo.err == nil { - return &openrtb_ext.ExtHttpCall{ - Uri: httpInfo.request.Uri, - RequestBody: string(httpInfo.request.Body), - ResponseBody: string(httpInfo.response.Body), - Status: httpInfo.response.StatusCode, - RequestHeaders: httpInfo.request.Headers, - } - } else if httpInfo.request == nil { - return &openrtb_ext.ExtHttpCall{} - } else { - return &openrtb_ext.ExtHttpCall{ - Uri: httpInfo.request.Uri, - RequestBody: string(httpInfo.request.Body), - RequestHeaders: httpInfo.request.Headers, + ext := &openrtb_ext.ExtHttpCall{} + + if httpInfo != nil && httpInfo.request != nil { + ext.Uri = httpInfo.request.Uri + ext.RequestBody = string(httpInfo.request.Body) + ext.RequestHeaders = httpInfo.request.Headers + + if httpInfo.err == nil && httpInfo.response != nil { + ext.ResponseBody = string(httpInfo.response.Body) + ext.Status = httpInfo.response.StatusCode } } + + return ext } // doRequest makes a request, handles the response, and returns the data needed by the diff --git a/exchange/bidder_test.go b/exchange/bidder_test.go index 8cc5b5e9383..7c01cd84c83 100644 --- a/exchange/bidder_test.go +++ b/exchange/bidder_test.go @@ -71,7 +71,6 @@ func TestSingleBidder(t *testing.T) { ctx = context.WithValue(ctx, DebugContextKey, true) for _, test := range testCases { - mockBidderResponse := &adapters.BidderResponse{ Bids: []*adapters.TypedBid{ { @@ -142,6 +141,48 @@ func TestSingleBidder(t *testing.T) { } } +func TestRequestBidRemovesSensitiveHeaders(t *testing.T) { + server := httptest.NewServer(mockHandler(200, "getBody", "responseJson")) + defer server.Close() + + requestHeaders := http.Header{} + requestHeaders.Add("Content-Type", "application/json") + requestHeaders.Add("Authorization", "anySecret") + + bidderImpl := &goodSingleBidder{ + httpRequest: &adapters.RequestData{ + Method: "POST", + Uri: server.URL, + Body: []byte("requestJson"), + Headers: requestHeaders, + }, + bidResponse: &adapters.BidderResponse{ + Bids: []*adapters.TypedBid{}, + }, + } + + debugInfo := &config.DebugInfo{Allow: true} + ctx := context.Background() + ctx = context.WithValue(ctx, DebugContextKey, true) + + bidder := adaptBidder(bidderImpl, server.Client(), &config.Configuration{}, &metricsConfig.DummyMetricsEngine{}, openrtb_ext.BidderAppnexus, debugInfo) + currencyConverter := currency.NewRateConverter(&http.Client{}, "", time.Duration(0)) + seatBid, errs := bidder.requestBid(ctx, &openrtb2.BidRequest{}, "test", 1, currencyConverter.Rates(), &adapters.ExtraRequestInfo{}, true) + + expectedHttpCalls := []*openrtb_ext.ExtHttpCall{ + { + Uri: server.URL, + RequestBody: "requestJson", + RequestHeaders: map[string][]string{"Content-Type": {"application/json"}}, + ResponseBody: "responseJson", + Status: 200, + }, + } + + assert.Empty(t, errs) + assert.ElementsMatch(t, seatBid.httpCalls, expectedHttpCalls) +} + // TestMultiBidder makes sure all the requests get sent, and the responses processed. // Because this is done in parallel, it should be run under the race detector. func TestMultiBidder(t *testing.T) { @@ -891,86 +932,160 @@ func TestMultiCurrencies_RequestCurrencyPick(t *testing.T) { } } -// TestBadResponseLogging makes sure that openrtb_ext works properly on malformed HTTP requests. -func TestBadRequestLogging(t *testing.T) { - info := &httpCallInfo{ - err: errors.New("Bad request"), - } - ext := makeExt(info) - if ext.Uri != "" { - t.Errorf("The URI should be empty. Got %s", ext.Uri) - } - if ext.RequestBody != "" { - t.Errorf("The request body should be empty. Got %s", ext.RequestBody) - } - if ext.ResponseBody != "" { - t.Errorf("The response body should be empty. Got %s", ext.ResponseBody) - } - if ext.Status != 0 { - t.Errorf("The Status code should be 0. Got %d", ext.Status) - } - if len(ext.RequestHeaders) > 0 { - t.Errorf("The request headers should be empty. Got %s", ext.RequestHeaders) - } -} - -// TestBadResponseLogging makes sure that openrtb_ext works properly if we don't get a sensible HTTP response. -func TestBadResponseLogging(t *testing.T) { - info := &httpCallInfo{ - request: &adapters.RequestData{ - Uri: "test.com", - Body: []byte("request body"), - Headers: http.Header{ - "header-1": []string{"value-1"}, +func TestMakeExt(t *testing.T) { + testCases := []struct { + description string + given *httpCallInfo + expected *openrtb_ext.ExtHttpCall + }{ + { + description: "Nil", + given: nil, + expected: &openrtb_ext.ExtHttpCall{}, + }, + { + description: "Empty", + given: &httpCallInfo{ + err: nil, + response: nil, + request: nil, }, + expected: &openrtb_ext.ExtHttpCall{}, + }, + { + description: "Request & Response - No Error", + given: &httpCallInfo{ + err: nil, + request: &adapters.RequestData{ + Uri: "requestUri", + Body: []byte("requestBody"), + Headers: makeHeader(map[string][]string{"Key1": {"value1", "value2"}}), + }, + response: &adapters.ResponseData{ + Body: []byte("responseBody"), + StatusCode: 999, + }, + }, + expected: &openrtb_ext.ExtHttpCall{ + Uri: "requestUri", + RequestBody: "requestBody", + RequestHeaders: map[string][]string{"Key1": {"value1", "value2"}}, + ResponseBody: "responseBody", + Status: 999, + }, + }, + { + description: "Request & Response - Error", + given: &httpCallInfo{ + err: errors.New("error"), + request: &adapters.RequestData{ + Uri: "requestUri", + Body: []byte("requestBody"), + Headers: makeHeader(map[string][]string{"Key1": {"value1", "value2"}}), + }, + response: &adapters.ResponseData{ + Body: []byte("responseBody"), + StatusCode: 999, + }, + }, + expected: &openrtb_ext.ExtHttpCall{ + Uri: "requestUri", + RequestBody: "requestBody", + RequestHeaders: map[string][]string{"Key1": {"value1", "value2"}}, + }, + }, + { + description: "Request Only", + given: &httpCallInfo{ + err: nil, + request: &adapters.RequestData{ + Uri: "requestUri", + Body: []byte("requestBody"), + Headers: makeHeader(map[string][]string{"Key1": {"value1", "value2"}}), + }, + response: nil, + }, + expected: &openrtb_ext.ExtHttpCall{ + Uri: "requestUri", + RequestBody: "requestBody", + RequestHeaders: map[string][]string{"Key1": {"value1", "value2"}}, + }, + }, { + description: "Response Only", + given: &httpCallInfo{ + err: nil, + response: &adapters.ResponseData{ + Body: []byte("responseBody"), + StatusCode: 999, + }, + }, + expected: &openrtb_ext.ExtHttpCall{}, }, - err: errors.New("Bad response"), - } - ext := makeExt(info) - if ext.Uri != info.request.Uri { - t.Errorf("The URI should be test.com. Got %s", ext.Uri) - } - if ext.RequestBody != string(info.request.Body) { - t.Errorf("The request body should be empty. Got %s", ext.RequestBody) - } - if ext.ResponseBody != "" { - t.Errorf("The response body should be empty. Got %s", ext.ResponseBody) } - if ext.Status != 0 { - t.Errorf("The Status code should be 0. Got %d", ext.Status) + + for _, test := range testCases { + result := makeExt(test.given) + assert.Equal(t, test.expected, result, test.description) } - assert.Equal(t, info.request.Headers, http.Header(ext.RequestHeaders), "The request headers should be \"header-1:value-1\"") } -// TestSuccessfulResponseLogging makes sure that openrtb_ext works properly if the HTTP request is successful. -func TestSuccessfulResponseLogging(t *testing.T) { - info := &httpCallInfo{ - request: &adapters.RequestData{ - Uri: "test.com", - Body: []byte("request body"), - Headers: http.Header{ - "header-1": []string{"value-1", "value-2"}, - }, +func TestRemoveSensitiveHeaders(t *testing.T) { + testCases := []struct { + description string + given http.Header + expected http.Header + }{ + { + description: "Nil", + given: nil, + expected: nil, }, - response: &adapters.ResponseData{ - StatusCode: 200, - Body: []byte("response body"), + { + description: "Empty", + given: http.Header{}, + expected: map[string][]string{}, + }, + { + description: "One", + given: makeHeader(map[string][]string{"Key1": {"value1"}}), + expected: makeHeader(map[string][]string{"Key1": {"value1"}}), + }, + { + description: "Many", + given: makeHeader(map[string][]string{"Key1": {"value1"}, "Key2": {"value2a", "value2b"}}), + expected: makeHeader(map[string][]string{"Key1": {"value1"}, "Key2": {"value2a", "value2b"}}), + }, + { + description: "Authorization Header Omitted", + given: makeHeader(map[string][]string{"authorization": {"secret"}}), + expected: http.Header{}, + }, + { + description: "Authorization Header Omitted - Case Insensitive", + given: makeHeader(map[string][]string{"AuThOrIzAtIoN": {"secret"}}), + expected: http.Header{}, + }, + { + description: "Authorization Header Omitted + Other Keys", + given: makeHeader(map[string][]string{"authorization": {"secret"}, "Key1": {"value1"}}), + expected: makeHeader(map[string][]string{"Key1": {"value1"}}), }, } - ext := makeExt(info) - if ext.Uri != info.request.Uri { - t.Errorf("The URI should be test.com. Got %s", ext.Uri) - } - if ext.RequestBody != string(info.request.Body) { - t.Errorf("The request body should be \"request body\". Got %s", ext.RequestBody) - } - if ext.ResponseBody != string(info.response.Body) { - t.Errorf("The response body should be \"response body\". Got %s", ext.ResponseBody) + + for _, test := range testCases { + removeSensitiveHeaders(test.given) + assert.Equal(t, test.expected, test.given, test.description) } - if ext.Status != info.response.StatusCode { - t.Errorf("The Status code should be 0. Got %d", ext.Status) +} + +func makeHeader(v map[string][]string) http.Header { + h := http.Header{} + for key, values := range v { + for _, value := range values { + h.Add(key, value) + } } - assert.Equal(t, info.request.Headers, http.Header(ext.RequestHeaders), "The request headers should be \"%s\". Got %s", info.request.Headers, ext.RequestHeaders) + return h } func TestMobileNativeTypes(t *testing.T) {