-
Notifications
You must be signed in to change notification settings - Fork 853
Description
#1756 introduced a really nice test for data races in bidder adapters. However, it doesn't detect when a slice field's underlying array gets modified. Such modifications can affect other bidders.
I think it's easiest to explain with an example. Consider the following request for Prebid Server:
POST http://localhost:8000/openrtb2/auction HTTP/1.1
Content-Type: application/json
{
"id" : "test-request-id",
"imp": [{
"id": "test-imp-id",
"tagid": "test-tagid",
"banner": {
"format": [{
"w": 300,
"h": 250
}]
},
"ext": {
"prebid": {
"bidder": {
"kobler": {},
"seedingAlliance": {
"adUnitId": "test-sa-adUnitId"
}
}
}
}
}],
"site": {
"id": "test-site-id"
},
"cur": ["test-cur-1", "test-cur-2"]
}
Focus on the "cur" field, which initially contains two elements, but then gets cut here:
prebid-server/endpoints/openrtb2/auction.go
Line 789 in be0a6b2
| req.Cur = req.Cur[0:1] |
After this operation, Cur is a slice of length 1 and capacity 2. This is not a problem, it's the standard way to cut a slice, but it is a requirement for the issue to surface.
Now let's look at two bidder adapters that manipulate the Cur field: kobler and seedingAlliance.
Kobler appends a "USD" currency to Cur:
prebid-server/adapters/kobler/kobler.go
Line 45 in be0a6b2
| sanitizedRequest.Cur = append(sanitizedRequest.Cur, supportedCurrency) |
SeedingAlliance appends an "EUR" currency to Cur:
| request.Cur = append(request.Cur, "EUR") |
Both are modifying the same underlying array. This data race can cause a bug where "EUR" is sent in the bid request to kobler or "USD" is sent to seedingAlliance.
But the focus of this issue is not on this specific bug with these two bidder adapters. This is just an example of a bug that's very easy to make. What I want to suggest in this issue is to enhance the data race test to detect such cases.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status