51degrees: extend device detection module with new fields

[justadreamer]

Type of change

• Feature

Description of change

51DegreesRtdProvider now populates device.hwv and introduces smarter logic populating device.model with a less specific HardwareNamePrefix property that would exclude specific device version. F.e. if HardwareName is iPhone 12 Pro Max, HardwareNamePrefix would be iPhone and HardwareNameVersion would be 12 Pro Max.
This PR should improve chances of correct targeting for bidders that expect a less specific model name and prefer receiving hardware version separately.

Other information

[antoxas1986]

Review: PR #4740 — 51degrees: extend device detection module with new fields

The intent is good but there are several correctness bugs that would silently corrupt bid requests in production.

---

[CRITICAL] HardwareName[0] is a byte, not a model name

File: hook_raw_auction_request.go, "model" closure

if len(fiftyOneDd.HardwareName) > 0 {
    return fiftyOneDd.HardwareName[0]   // BUG: returns uint8 byte value
}

HardwareName is a string, not a []string. Indexing a Go string with [0] yields a uint8 (e.g., 72 for "H"). This would write a numeric byte as device.model for every device that lacks both prefix and version — silent data corruption for every fallback case.

Suggested fix:

if fiftyOneDd.HardwareName != "" && fiftyOneDd.HardwareName != ddUnknown {
    return fiftyOneDd.HardwareName
}
return nil

---

[CRITICAL] hwv returns "" instead of nil, and hwv_legacy injects a non-spec field into every request

"hwv": func() any {
    if fiftyOneDd.HardwareNamePrefix != "" && fiftyOneDd.HardwareNameVersion != "" {
        return fiftyOneDd.HardwareNameVersion
    }
    return ""   // BUG: should be nil
},
"hwv_legacy": func() any {
    return ""   // BUG: unconditionally writes "hwv_legacy":"" into every request
},

The setMissingFields loop skips writing a field only when the closure returns nil. Returning "" bypasses this guard, so every device object missing hwv will now get "hwv": "" when the new fields are empty. hwv_legacy is not an OpenRTB field and must be removed.

Suggested fix:

"hwv": func() any {
    if fiftyOneDd.HardwareNameVersion != "" && fiftyOneDd.HardwareNameVersion != ddUnknown {
        return fiftyOneDd.HardwareNameVersion
    }
    return nil
},
// remove "hwv_legacy" entirely

---

[MAJOR] HardwareModel fallback silently dropped from model logic

The existing code tries HardwareModel first, falls back to HardwareName, and returns nil when neither is available. The new code skips HardwareModel entirely — any device that previously resolved via HardwareModel but lacks HardwareNamePrefix/HardwareNameVersion will get a corrupted or empty model.

Suggested fix — layer the new logic on top of the existing fallback chain:

"model": func() any {
    if fiftyOneDd.HardwareNamePrefix != "" && fiftyOneDd.HardwareNamePrefix != ddUnknown &&
        fiftyOneDd.HardwareNameVersion != "" && fiftyOneDd.HardwareNameVersion != ddUnknown {
        return fiftyOneDd.HardwareNamePrefix
    }
    newVal := fiftyOneDd.HardwareModel
    if newVal == ddUnknown {
        newVal = fiftyOneDd.HardwareName
    }
    if newVal != ddUnknown && newVal != "" {
        return newVal
    }
    return nil
},

---

[MAJOR] hwv guard requires both fields — version-only data is silently dropped

If HardwareNameVersion is populated but HardwareNamePrefix is empty, hwv currently returns "" and throws the version away. The hardware version value does not logically depend on whether the prefix is present. The fix above (checking only HardwareNameVersion) addresses this.

---

[MAJOR] New closures don't guard against the "Unknown" sentinel

The rest of setMissingFields consistently checks != ddUnknown. When a 51Degrees property has no match, getValue returns "Unknown" — not "". The new closures only check != "", so a device resolving HardwareNamePrefix = "Unknown" would pass the guard and set device.model = "Unknown" and device.hwv = "Unknown". The suggested fixes above include the != ddUnknown guards.

---

[MAJOR] Missing integration tests for the new behavior

• No test covers the happy path (both prefix + version populated) for hwv or the new model
• No test covers partial cases: prefix-only, version-only, or both empty
• The mockValues helper in device_info_extractor_test.go is not updated with the new properties — tests that exercise the new fields without it may panic on unexpected mock calls

Please add table-driven test cases in TestHandleRawAuctionHookEnrichment / TestHydrateFields covering at minimum:

1. Both HardwareNamePrefix + HardwareNameVersion populated → model = prefix, hwv = version
2. Only HardwareNameVersion populated → model falls back to HardwareModel, hwv = version
3. Neither populated → hwv absent from output, model uses existing fallback

---

[MINOR] model returns "" instead of nil as last resort

The existing code returned nil when no model could be determined, causing the field to be omitted. Returning "" now unconditionally writes "model": "" into every unresolvable device. This is a behavior change for all bidders. The suggested fix above uses return nil.

[justadreamer]

Hi @antoxas1986

Thanks for the careful read, but most of the claims in this review don't match the code that's actually in the PR (commit 9afa7ac9). Walking through them:

[CRITICAL] HardwareName[0] byte bug — not present. The model closure returns the string newVal, not HardwareName[0] (hook_raw_auction_request.go#L124-L130):

newVal := fiftyOneDd.HardwareModel
if newVal == ddUnknown {
    newVal = fiftyOneDd.HardwareName
}
if newVal != ddUnknown {
    return newVal
}
return nil

HardwareName is a string field on deviceInfo and it's returned as-is — there is no byte indexing anywhere.

[CRITICAL] hwv returns "" and hwv_legacy injected — not present. The hwv closure returns nil on the empty/unknown path (hook_raw_auction_request.go#L133-L138):

"hwv": func() any {
    if fiftyOneDd.HardwareNameVersion != "" && fiftyOneDd.HardwareNameVersion != ddUnknown {
        return fiftyOneDd.HardwareNameVersion
    }
    return nil
},

There is no hwv_legacy field anywhere in the file or the diff.

[MAJOR] HardwareModel fallback dropped — not dropped. Lines 124–130 preserve the original HardwareModel → HardwareName fallback chain; the new logic is layered on top, not in place of it.

[MAJOR] hwv guard requires both fields — it doesn't. The closure (line 134) only checks HardwareNameVersion. Version-only data is not dropped.

[MAJOR] New closures don't guard against ddUnknown — they do. Both the model prefix branch (line 120) and the hwv closure (line 134) check != ddUnknown alongside != "".

[MAJOR] Tests / mockValues not updated — they are:

• device_info_extractor_test.go#L110-L111 adds HardwareNamePrefix/HardwareNameVersion to mockValues.
• module_test.go updates TestHandleRawAuctionHookEnrichment, TestHandleRawAuctionHookEnrichmentWithErrors, and TestHydrateFields with both the new input fields and the expected "hwv":"Pro" in the output.

[MINOR] model returns "" instead of nil — not in the new code path. The new prefix branch returns nil when prefix is empty/unknown and falls through to the existing fallback, which has always had the != ddUnknown check.

On the design point in the suggested fix — gating model = HardwareNamePrefix on HardwareNameVersion also being present — that's intentionally not what we want. The intent is: try HardwareNamePrefix first; if it isn't available, fall back to HardwareModel/HardwareName. The presence of the version shouldn't decide whether a valid prefix gets used as the model.

Happy to add a few more table-driven cases for prefix-only / version-only / both-empty if that helps, but none of the correctness changes proposed here should land — they would either reintroduce real issues (silently dropping a valid prefix when version is missing) or change behavior to match a version of the code that wasn't actually committed. Could you double-check you were looking at 9afa7ac9 and not an earlier draft?