userId: Refactor PPID validation logic with official regex - #15411
Conversation
Updated PPID validation to use regex for length and character checks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d25d3ae15e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const regex = /^[0-9a-zA-Z+.=/_\-$,{}]{22,150}$/; | ||
| if (regex.test(ppidValue)) { |
There was a problem hiding this comment.
Validate the unmodified PPID before returning it
When the selected EID contains a character this new PPID regex permits (for example +, /, _, or =), the value has already been sanitized by replace(/[\W_]/g, '') before this test runs, so valid PPIDs can either be rejected after being shortened or returned to GAM as a different/colliding identifier. Validate the original ID, or stop stripping characters that the regex allows, before passing it to setPublisherProvidedId.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex if we stopped stripping characters the regex allows, that would break everyone expecting continuity of these ids
There was a problem hiding this comment.
Summary
- Preserved historical PPID continuity by continuing to strip non-alphanumeric characters before applying the updated 22–150 character validation. The validator now explicitly reflects the normalized alphanumeric value. modules/userId/index.tsL591-L603
- Added regression coverage confirming that Google-permitted special characters such as
+,/,_,=, and{}are normalized exactly as they were previously. test/spec/modules/userId_spec.jsL1095-L1109 - Committed the changes on
codex-preserve-ppid-normalizationwith commit647d670. - Created the follow-up PR titled “userId: Preserve PPID normalization continuity.”
Testing
- ✅
npx eslint modules/userId/index.ts test/spec/modules/userId_spec.js --cache --cache-strategy content - ✅
npx gulp test --nolint --file test/spec/modules/userId_spec.js— 139 affected tests passed in both feature configurations. - ✅
git diff --check - ✅
git status --short --branch— working tree clean after commit.
8144df0 to
b999570
Compare
Barecheck - Code coverage reportTotal: 96.66%Your code coverage diff: 0.00% ▴ |
Updated PPID validation to use regex for length and character checks.
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Google has a regex in the documentation for the format of the PPID that can be send to GAM.
This change uses that regex instead of the length-only check that was present in the code.
Other information
#15385 (related, but doesn't actually fix the issue)