Skip to content

Commit 419e651

Browse files
Fix GFP cookie header processing iteration logic. (#40290)
* Fix GFP cookie header processing iteration logic. * Unit test fixes. * Remove unused underscores. * Lint fix. --------- Co-authored-by: erwin mombay <[email protected]> (cherry picked from commit 23942ea)
1 parent f779ee8 commit 419e651

File tree

4 files changed

+76
-48
lines changed

4 files changed

+76
-48
lines changed

ads/google/a4a/cookie-utils.js

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,18 @@ export function maybeSetCookieFromAdResponse(win, fetchResponse) {
3232
try {
3333
cookiesToSet = JSON.parse(
3434
fetchResponse.headers.get(AMP_GFP_SET_COOKIES_HEADER_NAME)
35-
);
35+
)['cookie'];
3636
} catch {}
37+
if (!Array.isArray(cookiesToSet)) {
38+
return;
39+
}
3740
for (const cookieInfo of cookiesToSet) {
38-
const cookieName =
39-
(cookieInfo['_version_'] ?? 1) === 2 ? '__gpi' : '__gads';
40-
const value = cookieInfo['_value_'];
41+
const cookieName = (cookieInfo['version'] ?? 1) === 2 ? '__gpi' : '__gads';
42+
const value = cookieInfo['value'];
4143
// On proxy origin, we want cookies to be partitioned by subdomain to
4244
// prevent sharing across unrelated publishers, so we don't set a domain.
43-
const domain = getProxySafeDomain(win, cookieInfo['_domain_']);
44-
const expiration = Math.max(cookieInfo['_expiration_'], 0);
45+
const domain = getProxySafeDomain(win, cookieInfo['domain']);
46+
const expiration = Math.max(cookieInfo['expiration'], 0);
4547
setCookie(win, cookieName, value, expiration, {
4648
domain,
4749
secure: false,

ads/google/a4a/test/test-cookie-utils.js

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,49 @@ describes.fakeWin('#maybeSetCookieFromAdResponse', {amp: true}, (env) => {
1818
return;
1919
}
2020

21-
return JSON.stringify([
22-
{
23-
'_version_': 1,
24-
'_value_': 'val1',
25-
'_domain_': 'foo.com',
26-
'_expiration_': Date.now() + 100_000,
27-
},
28-
{
29-
'_version_': 2,
30-
'_value_': 'val2',
31-
'_domain_': 'foo.com',
32-
'_expiration_': Date.now() + 100_000,
33-
},
34-
]);
21+
return JSON.stringify({
22+
'cookie': [
23+
{
24+
'version': 1,
25+
'value': 'val1',
26+
'domain': 'foo.com',
27+
'expiration': Date.now() + 100_000,
28+
},
29+
{
30+
'version': 2,
31+
'value': 'val2',
32+
'domain': 'foo.com',
33+
'expiration': Date.now() + 100_000,
34+
},
35+
],
36+
});
3537
},
3638
},
3739
});
3840

3941
expect(getCookie(env.win, '__gads')).to.equal('val1');
4042
expect(getCookie(env.win, '__gpi')).to.equal('val2');
4143
});
44+
45+
it('should not throw for malformed JSON', () => {
46+
expect(
47+
() =>
48+
void maybeSetCookieFromAdResponse(env.win, {
49+
headers: {
50+
has: (header) => {
51+
return header === AMP_GFP_SET_COOKIES_HEADER_NAME;
52+
},
53+
get: (header) => {
54+
if (header !== AMP_GFP_SET_COOKIES_HEADER_NAME) {
55+
return;
56+
}
57+
58+
return JSON.stringify({});
59+
},
60+
},
61+
})
62+
).not.to.throw();
63+
});
4264
});
4365

4466
describes.fakeWin('#handleCookieOptOutPostMessage', {amp: true}, (env) => {

extensions/amp-ad-network-adsense-impl/0.1/test/test-amp-ad-network-adsense-impl.js

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,20 +1724,22 @@ describes.fakeWin(
17241724
return;
17251725
}
17261726

1727-
return JSON.stringify([
1728-
{
1729-
'_version_': 1,
1730-
'_value_': 'val1',
1731-
'_domain_': 'foo.com',
1732-
'_expiration_': Date.now() + 100_000,
1733-
},
1734-
{
1735-
'_version_': 2,
1736-
'_value_': 'val2',
1737-
'_domain_': 'foo.com',
1738-
'_expiration_': Date.now() + 100_000,
1739-
},
1740-
]);
1727+
return JSON.stringify({
1728+
'cookie': [
1729+
{
1730+
'version': 1,
1731+
'value': 'val1',
1732+
'domain': 'foo.com',
1733+
'expiration': Date.now() + 100_000,
1734+
},
1735+
{
1736+
'version': 2,
1737+
'value': 'val2',
1738+
'domain': 'foo.com',
1739+
'expiration': Date.now() + 100_000,
1740+
},
1741+
],
1742+
});
17411743
},
17421744
},
17431745
});

extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1919,20 +1919,22 @@ for (const {config, name} of [
19191919
return;
19201920
}
19211921

1922-
return JSON.stringify([
1923-
{
1924-
'_version_': 1,
1925-
'_value_': 'val1',
1926-
'_domain_': 'foo.com',
1927-
'_expiration_': Date.now() + 100_000,
1928-
},
1929-
{
1930-
'_version_': 2,
1931-
'_value_': 'val2',
1932-
'_domain_': 'foo.com',
1933-
'_expiration_': Date.now() + 100_000,
1934-
},
1935-
]);
1922+
return JSON.stringify({
1923+
'cookie': [
1924+
{
1925+
'version': 1,
1926+
'value': 'val1',
1927+
'domain': 'foo.com',
1928+
'expiration': Date.now() + 100_000,
1929+
},
1930+
{
1931+
'version': 2,
1932+
'value': 'val2',
1933+
'domain': 'foo.com',
1934+
'expiration': Date.now() + 100_000,
1935+
},
1936+
],
1937+
});
19361938
},
19371939
},
19381940
});

0 commit comments

Comments
 (0)