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

Remove Bidding Signals Format "V1" support #1390

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions spec.bs
Original file line number Diff line number Diff line change
Expand Up @@ -2658,8 +2658,6 @@ To <dfn>fetch WebAssembly</dfn> given a [=URL=] |url| and an [=environment setti

The <dfn http-header><code>Data-Version</code></dfn> HTTP response header is a
[=structured header=] whose value must be an [=structured header/integer=].
The <dfn http-header><code>X-fledge-bidding-signals-format-version</code></dfn> HTTP response header
is a [=structured header=] whose value must be an [=structured header/integer=].

<div algorithm>
To <dfn>fetch trusted signals</dfn> given a [=URL=] |url|, an [=origin=] |scriptOrigin|, a
Expand Down Expand Up @@ -2690,7 +2688,6 @@ To <dfn>fetch trusted signals</dfn> given a [=URL=] |url|, an [=origin=] |script

1. Let |signals| be null.
1. Let |dataVersion| be null.
1. Let |formatVersion| be null.
1. Let |perInterestGroupData| be an [=ordered map=] whose [=map/keys=] are [=interest group/name=]
[=strings=] and whose [=map/values=] are [=bidding signals per interest group data=].
1. [=Fetch=] |request| with [=fetch/useParallelQueue=] set to true, and
Expand All @@ -2704,20 +2701,16 @@ To <dfn>fetch trusted signals</dfn> given a [=URL=] |url|, an [=origin=] |script
1. If |dataVersion| is not null:
1. If |dataVersion| is not an integer, or is less than 0 or more than 2<sup>32</sup>&minus;1,
set |signals| to failure and return.
1. If |isBiddingSignal| is true, then set |formatVersion| to the result of
[=header list/getting a structured field value=] given
[:X-fledge-bidding-signals-format-version:] and "`item`" from |headers|.
1. Set |signals| to the result of [=parsing JSON bytes to an Infra value=] |responseBody|.
1. Wait for |signals| to be set.
1. If |signals| is a parsing exception, or if |signals| is not an [=ordered map=], return « null,
null, null ».
1. If |formatVersion| is 2:
1. If |signals|["`keys`"] does not [=map/exist=], return « null, null ».
1. Set |signals| to |signals|["`keys`"].
1. If |signals| is not an [=ordered map=], return « null, null ».
1. If |signals|["`perInterestGroupData`"] [=map/exists=] and is an [=ordered map=]:
1. [=Assert=] |isBiddingSignal| is true.
1. Let |perInterestGroupData| be |signals|["`perInterestGroupData`"].
1. If |signals|["`keys`"] does not [=map/exist=], return « null, null ».
1. Set |signals| to |signals|["`keys`"].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little awkward, and this entire method has a bunch of bugs (e.g., line 2711 should be before we set signals, a bunch of the returns only return two values instead of three, some places "set |signals| to null and return failure" (signals isn't even being returned, and wasn't passed in by the caller, so setting it to null doesn't do anything), etc.

I think fixing all of that is beyond the scope of this PR.

1. If |signals| is not an [=ordered map=], return « null, null ».
1. If |signals|["`perInterestGroupData`"] [=map/exists=] and is an [=ordered map=]:
1. [=Assert=] |isBiddingSignal| is true.
1. Let |perInterestGroupData| be |signals|["`perInterestGroupData`"].
1. [=map/For each=] |key| → |value| of |signals|:
1. [=map/Set=] |signals|[|key|] to the result of [=serializing an Infra value to a JSON string=]
given |value|.
Expand Down