Triplelift Native: fix panic

[guscarreon]

This pull request adds an extra nil check to the logic recently added in #3745 as it was triggering panics when a nil value ofsiteCopy.Publisher would get dereferenced in line 78 of adapters/triplelift_native/triplelift_native.go.

73     if ext.Data != nil {
74         extData = *ext.Data
75     }
76
77     if extData.TagCode != "" {
78         if siteCopy.Publisher.Domain == "msn.com" { //<-- PANIC
79             imp.TagID = extData.TagCode
80         } else {
81             imp.TagID = tlext.InvCode
82         }
83     } else {
84         imp.TagID = tlext.InvCode
85     }
adapters/triplelift_native/triplelift_native.go [RO]

@triplelift, we were wondering if:

1. Do you agree with this change?

2. Given that the imp.TagID can now come from either ext.Data.TagCode or tlext.InvCode, do we still want to return error in line 64 or should we remove?

   57       if err := json.Unmarshal(ext.Bidder, &tlext); err != nil {
   58           return err
   59       }
   60       if imp.Native == nil {
   61           return fmt.Errorf("no native object specified")
   62       }
   63 -     if tlext.InvCode == "" {
   64 -         return fmt.Errorf("no inv_code specified")
   65 -     }
   66  
   67       if ext.Data != nil && len(ext.Data.TagCode) > 0 && request.Site != nil && request.Site.Publisher != nil && request.Site.Publisher.Domain == "msn.com" {
   68           imp.TagID = ext.Data.TagCode
   69       } else {
   70           imp.TagID = tlext.InvCode
   71       }
   72  
   73       // floor is optional
   74       if tlext.Floor == nil {
   75           return nil
   76       }
   adapters/triplelift_native/triplelift_native.go [RO]
   

3. If request.Site is nil, are we interested in using the App.Publisher.Domain instead? Function effectivePubID downstream makes use of App.Publisher values. Should processImp use App.Publisher.Domain if available?

[github-actions]

Code coverage summary

Note:

• Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
• Coverage summary encompasses all commits leading up to the latest one, caf5b28

triplelift_native

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:45:	getBidType		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:49:	processImp		94.1%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:83:	effectivePubID		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:99:	MakeRequests		92.9%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:144:	getPublisher		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:151:	getBidCount		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:159:	MakeBids		94.7%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:196:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:215:	getExtraInfo		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:228:	getDefaultExtraInfo	100.0%
total:											(statements)		95.8%

[github-actions]

Code coverage summary

Note:

• Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
• Coverage summary encompasses all commits leading up to the latest one, 618a519

triplelift_native

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:45:	getBidType		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:49:	processImp		94.1%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:82:	effectivePubID		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:98:	MakeRequests		92.9%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:143:	getPublisher		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:150:	getBidCount		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:158:	MakeBids		94.7%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:195:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:214:	getExtraInfo		100.0%
github.com/prebid/prebid-server/v2/adapters/triplelift_native/triplelift_native.go:227:	getDefaultExtraInfo	100.0%
total:											(statements)		95.8%

[guscarreon]

Copying request.Site into siteCopy and ext.Data into extData created overhead and, given that the processImp function is only interested in reading and not overwriting these values, no shared memory issues will come if we just use the originals. Let me know if you agree with their removal.

[bsardo]

We've opened two PRs for the same problem. Closing as we already have #3824. These questions have been asked offline.