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

amp-analytics does not accept query string parameters with 2 _'s in them #25981

Closed
mborof opened this issue Dec 11, 2019 · 4 comments
Closed

Comments

@mborof
Copy link
Contributor

mborof commented Dec 11, 2019

What's the issue?

When using amp-analytics if I build a request that has a query string parameter with 2 _'s in it then amp-analytics does not generate any requests.

For example, this generates an HTTP post:

const VENDOR_CONFIG = jsonLiteral({
'requests': {
// Variables
'req' : 'https://pubads.g.doubleclick.net/subopt?',
},
triggers: {
on: {
'on': 'visible',
'request': 'req',
},
'transport': {
'beacon': true,
'xhrpost': false,
'image': false,
},
});

And this does not generate an HTTP post:

const VENDOR_CONFIG = jsonLiteral({
'requests': {
// Variables
'req': 'https://pubads.g.doubleclick.net/subopt?__amp_source_origin=${sourceHost}',
},
triggers: {
on: {
'on': 'visible',
'request': 'req',
},
'transport': {
'beacon': true,
'xhrpost': false,
'image': false,
},
});

How do we reproduce the issue?

  1. Create a JavaScript file with the above in it inside the amp-analytics vendors folder.
  2. Register the component in vendors.js (remember the TYPE)
  3. Add this tag to an amp page (filling in the TYPE):
4. Execute the page you added it to. You should see a request generated without the __ query string param and not see one generated with it.

What browsers are affected?

I was specifically testing in Chrome; I did not test other browsers.

Which AMP version is affected?

This is the current version of AMP (as of 12/11/2019). I have no seen this work previously (or tried).

@micajuine-ho
Copy link
Contributor

Hi @mborof, thank you for your issue report.

Are you still experiencing this issue?

I tried to reproduce this issue, through both inline config as well as altering a vendor config, without any success.

@mborof
Copy link
Contributor Author

mborof commented Jan 2, 2020

Yes. In this PR:

#26003

If I modify "subscriptions-propensity.json" so that _amp_source_origin= is instead __amp_source_origin I no longer get any amp-analytics HTTP requests from my propensity module.

@micajuine-ho
Copy link
Contributor

@mborof Thanks for your follow up.

The reason for this is that __amp_source_origin is not allowed as a parameter name and not that it has 2 leading underscores. See #4670 for more details.

@mborof
Copy link
Contributor Author

mborof commented Jan 2, 2020

Makes sense. Closing.

@mborof mborof closed this as completed Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants