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

Spec "Referer" and "Origin" headers in reporting beacons. #164

Merged
merged 10 commits into from
Oct 2, 2024

Conversation

blu25
Copy link
Collaborator

@blu25 blu25 commented Jun 3, 2024

This PR specs the following behavior:

  • Setting the "Origin" header to the origin of the frame that invoked reportEvent() for destination URL events.
  • Setting the "Origin" header to the worklet's origin (which we are calling reporting url declarer origin) for destination enum and automatic beacon events.
  • Setting the "Referer" header to the ad frame root's origin (which we are calling initiator ancestor root origin) for automatic beacons triggered by component ad frames.
  • Setting the "Referer" header to the origin of the frame that triggered the beacon for all beacons triggered not by component ads.

See this document for notes/reasonings on why things were done the way they were.

There will need to be a separate effort in the Protected Audience spec to set some of the values that these changes are expecting to be set. More specifically:


Preview | Diff

@blu25 blu25 requested a review from domfarolino June 3, 2024 22:15
@blu25 blu25 marked this pull request as draft June 4, 2024 21:39
@domfarolino
Copy link
Collaborator

Is this a draft or is this ready for my review?

@blu25
Copy link
Collaborator Author

blu25 commented Jun 5, 2024

Is this a draft or is this ready for my review?

It's back to draft after changes were made during the impl's code review. I'll re-request when it's ready for you to take a look.

@blu25 blu25 marked this pull request as ready for review June 6, 2024 21:59
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
@domfarolino
Copy link
Collaborator

Any idea why the latest preview does not seem to be updated? In the preview "declarer origin" can be null but I don't think that matches the spec text, right?

@blu25
Copy link
Collaborator Author

blu25 commented Jul 11, 2024

Any idea why the latest preview does not seem to be updated? In the preview "declarer origin" can be null but I don't think that matches the spec text, right?

I'm not sure. It seems to be working again since I uploaded the latest patch, so you should be good to look at the preview/diff.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@blu25
Copy link
Collaborator Author

blu25 commented Sep 20, 2024

Let's hold off on merging this until the latest discussion in the design doc is addressed.

spec.bs Outdated Show resolved Hide resolved
spec.bs Outdated Show resolved Hide resolved
@domfarolino domfarolino merged commit 1454a61 into master Oct 2, 2024
2 checks passed
@domfarolino domfarolino deleted the liam-referer-origin-header branch October 2, 2024 00:18
github-actions bot added a commit that referenced this pull request Oct 2, 2024
SHA: 1454a61
Reason: push, by domfarolino

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.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