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

attributionsrc set by window.open is broken with respect to URL parameters #407

Closed
apasel422 opened this issue May 4, 2022 · 4 comments
Closed
Assignees

Comments

@apasel422
Copy link
Collaborator

apasel422 commented May 4, 2022

The inability to use = within a feature parameter for window.open is a limitation of both the specification and Chromium's implementation. For example, attributionsrc=https://a.test?x=y&h=j gets interpreted as:

attributionsrc = https://a.test?x
y&h = j

Some solutions we could consider:

  1. Modify the window.open spec and implementation to permit escaping/quoting of values.
  2. Require attributionsrc feature values to be URL-encoded and perform URL-decoding on them before using them.
  3. Stop using feature strings altogether and specify attributionsrc via a separate parameter to window.open.

Originally reported in https://groups.google.com/a/chromium.org/g/attribution-reporting-api-dev/c/_Q_dYzbSo38.

@alois-bissuel
Copy link
Contributor

Thanks for tracking this here.
All mentioned solutions are fine for us (with maybe a preference for 2 and 3).
Meanwhile we will encode the query part of the URL, so that the equal signs are escaped, and we will decode it in our endpoint.

@apasel422
Copy link
Collaborator Author

I am leaning toward option 2 myself, as it doesn't require option 1's modification of the HTML spec (with the backwards-compatibility concerns that might entail) and doesn't require adding a completely separate API surface as in option 3.

@csharrison
Copy link
Collaborator

I agree 2 seems like the simplest option. 3 was how the feature was originally implemented and there was a whole host of compat issues with adding more params to window.open.

@apasel422 apasel422 self-assigned this May 4, 2022
@maudnals
Copy link
Contributor

maudnals commented May 4, 2022

2 seems OK to me too, I can gather additional input regarding developer experience to confirm

aarongable pushed a commit to chromium/chromium that referenced this issue May 5, 2022
Otherwise, attributionsrc URLs containing special characters such as '='
are incorrectly truncated.

WICG/attribution-reporting-api#407
WICG/attribution-reporting-api#408

Bug: 1322450
Change-Id: I6d861c2744f8be048633a6bc00305a4fc09ab73c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3626039
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Quick-Run: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999949}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Otherwise, attributionsrc URLs containing special characters such as '='
are incorrectly truncated.

WICG/attribution-reporting-api#407
WICG/attribution-reporting-api#408

Bug: 1322450
Change-Id: I6d861c2744f8be048633a6bc00305a4fc09ab73c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3626039
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Quick-Run: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: John Delaney <johnidel@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#999949}
NOKEYCHECK=True
GitOrigin-RevId: 0a6664786e459a221e71cf6788f9e40aeb4499be
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

No branches or pull requests

4 participants