Skip to content

Remove duplication of partitionKey that resulted in invalid json#2826

Closed
igvk wants to merge 3 commits intohardkoded:masterfrom
igvk:master
Closed

Remove duplication of partitionKey that resulted in invalid json#2826
igvk wants to merge 3 commits intohardkoded:masterfrom
igvk:master

Conversation

@igvk
Copy link

@igvk igvk commented Nov 17, 2024

Cookies having Partition Keys are serialized incorrectly leading to errors when sending such cookies to the browser due to invalid json.

@kblok
Copy link
Member

kblok commented Nov 17, 2024

Do you wan to add some tests?

@igvk
Copy link
Author

igvk commented Nov 17, 2024

Yeah, it's a pity there are no tests on cookie partitioning.
I'll see what I can do.

@kblok
Copy link
Member

kblok commented Nov 17, 2024

Yeah, it's a pity there are no tests on cookie partitioning.

I'll see what I can do.

At least testing the de/serialization

@igvk
Copy link
Author

igvk commented Nov 17, 2024

Here is the test of partition cookie

kblok added a commit that referenced this pull request Feb 16, 2026
The CookiePartitionKeyConverter was writing PartitionKey as a plain string,
but Chrome expects an object with topLevelSite and hasCrossSiteAncestor
properties. This caused "Invalid cookie fields" errors when setting cookies
with a partition key. Also adds the missing ShouldSetACookieWithAPartitionKey
test from upstream.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kblok
Copy link
Member

kblok commented Feb 16, 2026

Closing in favor of #3144

@kblok kblok closed this Feb 16, 2026
kblok added a commit that referenced this pull request Feb 18, 2026
* Fix CookiePartitionKey serialization and add partition key test (#2826)

The CookiePartitionKeyConverter was writing PartitionKey as a plain string,
but Chrome expects an object with topLevelSite and hasCrossSiteAncestor
properties. This caused "Invalid cookie fields" errors when setting cookies
with a partition key. Also adds the missing ShouldSetACookieWithAPartitionKey
test from upstream.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix serialization test to expect object format for PartitionKey

The converter now serializes PartitionKey as {topLevelSite, hasCrossSiteAncestor}
object, so the test assertion needs to match.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Support partition key cookies in BiDi and fix Firefox compatibility

- Remove the "BiDi only allows domain partition keys" exception for
  string partition keys, matching upstream Puppeteer behavior
- Use StorageKeyPartitionDescriptor with sourceOrigin when a partition
  key is provided, BrowsingContextPartitionDescriptor otherwise
- Extract goog:partitionKey and goog:sourceScheme from BiDi cookie
  extension data when reading cookies back
- Make SourceScheme assertion Chrome-only since Firefox native BiDi
  doesn't include goog: extension properties
- Add local test expectation to mark partition key test as FAIL on
  Firefox (native BiDi lacks goog:partitionKey)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments