setUninstallURL: String must not be more than 255 characters long.

[greiner]

Environment

Chrome 70
Adblock Plus 3.4.2.2206

How to reproduce

Open background page console.

Observed behaviour

The following error is shown:

Uncaught (in promise) Error: Invalid value for argument 1. String must not be more than 255 characters long.
    at validate (extensions::schemaUtils:34)
    at Object.normalizeArgumentsAndValidate (extensions::schemaUtils:119)
    at Object.<anonymous> (extensions::binding:363)
    at Object.value [as setUninstallURL] (polyfill.js:108)
    at Object.exports.setUninstallURL (uninstall.js:60)
    at Promise.all.then.then.then.then (subscriptionInit.js:311)

Expected behaviour

No such error is shown.

Notes

The uninstall URL it tries to set has 256 characters:

https://adblockplus.org/redirect
  ?link=uninstalled
  &lang=en-US
  &addonName=adblockpluschrome
  &addonVersion=3.4.2.2206
  &application=chrome
  &applicationVersion=70.0.3538.110
  &platform=chromium
  &platformVersion=70.0.3538.110
  &notificationDownloadCount=30-89
  &corrupted=0

What to change

Since we need to keep the URL under 255 characters, we need to keep the key names as short as possible. For #6655 where we ended up shortening the "dataCorrupted" parameter to "corrupted", but this time we need to do more.

Of the submitted query parameters, we're stuck with (for now) the two which are used to determine the page being viewed; link and lang.

But the others, we can do something about. Let's use these abbreviations:

Current name	Abbreviation
addonName	an
addonVersion	av
application	ap
applicationVersion	apv
platform	p
platformVersion	pv
notificationDownloadCount	ndc
corrupted	c

Which brings the number of characters used by the key names down to 24, which by Thomas' calculations means the total URL length will now be between 119 and 192 characters.

Hints for testers

Uninstall Adblock Plus and look at the URL for the uninstall page which opens. Ensure that the parameters are all passed correctly, using the abbreviations listed above.

[greiner]

Looking through the code, I came up with the following values:
assuming application and platform versions follow the pattern /.{2,4}(\.{1,4}){0,3}/

	Separators	Key	Value {min,max}
​https://adblockplus.org/redirect	0	32	0
link	2	4	9
lang	2	4	{2,6}
addonName	2	9	{15,18}
addonVersion	2	12	{1,23}
application	2	11	{5,7+}
applicationVersion	2	18	{2?,19?}
platform	2	8	{5,8}
platformVersion	2	15	{2?,19?}
notificationDownloadCount	2	25	{1,6}
corrupted	2	9	1
total	20	147	{43?,116?+}	{210?,283?+}

Example of a string with 283 characters:

https://adblockplus.org/redirect
  ?link=uninstall
  &lang=es_419
  &addonName=adblockplusfirefox
  &addonVersion=10000.10000.10000.10000
  &application=firefox
  &applicationVersion=1000.1000.1000.1000
  &platform=chromium
  &platformVersion=1000.1000.1000.1000
  &notificationDownloadCount=90-179
  &corrupted=0

That leads me to think that compressing each of the keys to only three characters could be sufficient for now and would still leave us with a bit of room to expand.

[kzar]

That leads me to think that compressing each of the keys to only three characters could be sufficient for now and would still leave us with a bit of room to expand.

I wouldn't mind doing that as long as the code still makes it clear what the values we submit are, for example with comments or otherwise making it obvious. E.g. // dc = dataCorrupted?. I mention this because AdBlock doesn't, I remember reading through their code to figure out which bits of data they were submitting. I don't want to give the impression that we're hiding something, or doing something shady.

[kzar]

Copying in some data folks. What do you think, would it be acceptable to replace the key names with one/two letter abbreviations? If so, any preferences what they should be?

[sporz]

Acknowledged - we can work with any abbreviation you come up with. Please give us a heads up before it goes live, so we can adjust our processing.

[kzar]

Alright, how about this?

[kzar]

With #6840 making it impossible to send uppercase query string parameters on Edge, I wonder if the parameter name abbreviations we pick should all be lowercase? So far I used capitalisation in an attempt to make them easier to understand.

[kzar]

I've gone ahead and triaged this, and implemented it. Please, if you have any problem with the abbreviations I've picked speak now!

[sebastian]

Replying to kzar:

With #6840 making it impossible to send uppercase query string parameters on Edge, I wonder if the parameter name abbreviations we pick should all be lowercase? So far I used capitalisation in an attempt to make them easier to understand.

+1. That's the same I was thinking when I read this issue's description.

[kzar]

Replying to sebastian:

+1. That's the same I was thinking when I read this issue's description.

Cool alright, I'll make them all lowercase.

[abpbot]

A commit referencing this issue has landed:
​Issue 7153 - Abbreviate the parameter names in uninstall URL