Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Housekeeping: Update 3rd party dependencies #3786

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

JimTharioAmazon
Copy link

@JimTharioAmazon JimTharioAmazon commented Feb 25, 2025

🔧 Type of changes

  • new bid adapter
  • bid adapter update
  • new feature
  • new analytics adapter
  • new module
  • module update
  • bugfix
  • documentation
  • configuration
  • dependency update
  • tech debt (test coverage, refactorings, etc.)

✨ What's the context?

  • Updated three dependencies to address several CVEs and improve security posture: geoip2, json-schema-validator, and pinned commons-logging to a safe version.
  • Updated unit tests for geoip2 and json-schema-validator for dependency updates.
    • Updated geoip2 removed several constructors requiring more mocking for the Geo types.
    • Updated json-schema-validator returns an additional message for some failed validations, ""$: should be valid to one and only one schema, but 0 are valid"

🧠 Rationale behind the change

Update dependencies to improve security posture.

🔎 New Bid Adapter Checklist

  • verify email contact works
  • NO fully dynamic hostnames
  • geographic host parameters are NOT required
  • direct use of HTTP is prohibited - implement an existing Bidder interface that will do all the job
  • if the ORTB is just forwarded to the endpoint, use the generic adapter - define the new adapter as the alias of the generic adapter
  • cover an adapter configuration with an integration test

🧪 Test plan

  • Relied on existing unit tests for verification of changes.
  • Used mvn clean package for build and test after each change.

🏎 Quality check

  • [Y] Are your changes following our code style guidelines?
  • [N] Are there any breaking changes in your code?
  • [-] Does your test coverage exceed 90%?
  • [N] Are there any erroneous console logs, debuggers or leftover code in your changes?

@JimTharioAmazon JimTharioAmazon marked this pull request as ready for review February 25, 2025 17:17
@Net-burst Net-burst requested a review from osulzhenko February 25, 2025 17:19
@osulzhenko osulzhenko added dependencies Pull requests that update a dependency file do not port labels Feb 26, 2025
@Net-burst Net-burst changed the title Update 3rd party dependencies Housekeeping: Update 3rd party dependencies Feb 26, 2025
Copy link
Collaborator

@osulzhenko osulzhenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, no Maven conflicts or issues. But could you please also update a functional test: PBS should validate request as alias request and emit proper warnings when validation fails for request?
The error message changed due to an update in the json-schema-validator dependency to:

        and: "Bid response should contain warning"
        assert bidResponse.ext?.warnings[PREBID]?.code == [999, 999]
        assert bidResponse.ext?.warnings[PREBID]*.message ==
                ["WARNING: request.imp[0].ext.prebid.bidder.${APPNEXUS.value} was dropped with a reason: request.imp[0].ext.prebid.bidder.${APPNEXUS.value} failed validation.\n" +
                         "\$: must be valid to one and only one schema, but 0 are valid\n" +
                         "\$: required property 'placement_id' not found\n" +
                         "\$: required property 'inv_code' not found\n" +
                         "\$: required property 'placementId' not found\n" +
                         "\$: required property 'member' not found\n" +
                         "\$: required property 'invCode' not found",
                         "WARNING: request.imp[0].ext must contain at least one valid bidder"]

@JimTharioAmazon
Copy link
Author

Hi, updated the PR with the functional test change. Thanks for reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file do not port
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants