Skip to content

Commit 1ef5cf7

Browse files
Baohua Shanmeta-codesync[bot]
authored andcommitted
Validate user id propagated from upstream
Summary: Even though we already added some validation on the places setting user id to context prop, however based on https://fburl.com/unidash/4qwfs4gr, we still see there are some invalid user id in context prop. Based on analysis, this could be passed through context prop propagation. Add the validation to block invalid user id from upstream. Differential Revision: D84106032 fbshipit-source-id: e310ceabd7a95e3cd7b3c894de938704abc8951f
1 parent 21b115f commit 1ef5cf7

2 files changed

Lines changed: 18 additions & 12 deletions

File tree

third-party/thrift/src/thrift/lib/hack/src/ThriftContextPropState.php

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,21 @@ private static function getTfmFromString(
159159
if ($skip_experiment_id_ingestion && $tfm->experiment_ids is nonnull) {
160160
$tfm->experiment_ids = vec[];
161161
}
162+
// Make sure the user ids are valid (non-null, positive) and matches the type
163+
$user_ids = $tfm->baggage?->user_ids;
164+
if ($user_ids is nonnull) {
165+
if ($user_ids->fb_user_id is nonnull) {
166+
$user_ids->fb_user_id =
167+
self::coerceId($user_ids->fb_user_id, UserIdCategory::FB);
168+
}
169+
if ($user_ids->ig_user_id is nonnull) {
170+
$user_ids->ig_user_id =
171+
self::coerceId($user_ids->ig_user_id, UserIdCategory::IG);
172+
}
173+
}
174+
if ($tfm->baggage is nonnull) {
175+
$tfm->baggage->user_ids = $user_ids;
176+
}
162177

163178
return $tfm;
164179

@@ -464,18 +479,6 @@ public function getUserIds()[]: ?ContextProp\UserIds {
464479
return ($ig_user_id as int);
465480
}
466481

467-
// user id setters
468-
private function setUserIds(
469-
?ContextProp\UserIds $user_ids,
470-
)[write_props]: void {
471-
$this->storage->baggage =
472-
$this->storage->baggage ?? ContextProp\Baggage::withDefaultValues();
473-
474-
$baggage = $this->storage->baggage as nonnull;
475-
$baggage->user_ids = $user_ids;
476-
$this->dirty();
477-
}
478-
479482
private function setFBUserId(?int $fb_user_id): void {
480483
$this->storage->baggage =
481484
$this->storage->baggage ?? ContextProp\Baggage::withDefaultValues();

third-party/thrift/src/thrift/lib/hack/src/__tests__/ThriftContextPropStateTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ public function testInitialization()[defaults]: void {
203203

204204
public async function testInitializationWithUserIds(
205205
)[defaults]: Awaitable<void> {
206+
self::mockFunction(IgidUtils::isUserFbid<>)->mockReturn(true);
206207
$tfm = ThriftFrameworkMetadata::withDefaultValues();
207208
$tfm->baggage = ContextProp\Baggage::withDefaultValues();
208209
$tfm->baggage->user_ids = ContextProp\UserIds::fromShape(
@@ -253,6 +254,7 @@ public function testInitialization()[defaults]: void {
253254
public async function testInitializationWithBothTFMandVC(
254255
)[defaults]: Awaitable<void> {
255256
MockJustKnobs::setBool('meta_cp/www:enable_user_id_ctx_prop', true);
257+
self::mockFunction(IgidUtils::isUserFbid<>)->mockReturn(true);
256258
$tfm = ThriftFrameworkMetadata::withDefaultValues();
257259
$tfm->baggage = ContextProp\Baggage::withDefaultValues();
258260
$tfm->baggage->user_ids = ContextProp\UserIds::fromShape(
@@ -309,6 +311,7 @@ public function testInitialization()[defaults]: void {
309311
public async function testUpdatedWithExplicitFBUserId(
310312
)[defaults]: Awaitable<void> {
311313
MockJustKnobs::setBool('meta_cp/www:enable_user_id_ctx_prop', true);
314+
self::mockFunction(IgidUtils::isUserFbid<>)->mockReturn(true);
312315
$tfm = ThriftFrameworkMetadata::withDefaultValues();
313316
$tfm->baggage = ContextProp\Baggage::withDefaultValues();
314317
$tfm->baggage->user_ids = ContextProp\UserIds::fromShape(

0 commit comments

Comments
 (0)