1437585 - MAX_ADVISABLE_PREF_LENGTH is too small for unit test network.proxy.autoconfig_url

Status: RESOLVED WONTFIX

(Core :: Preferences: Backend, defect)

Description

[Bob Clary [:bc] (inactive)]

https://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#123 sets MAX_ADVISABLE_PREF_LENGTH to 4K with the expectation that this length should be sufficient for everyone.

https://searchfox.org/mozilla-central/source/modules/libpref/Preferences.cpp#1769 warns that a pref which exceeds MAX_ADVISABLE_PREF_LENGTH will not be sent to content processes.

I have run into a case running Mochitest mda where the network.proxy.autoconfig_url exceeds 4K. For example,

https://treeherder.mozilla.org/logviewer.html#?job_id=160781451&repo=try&lineNumber=30598

"I/GeckoConsole(  740): Warning: attempting to write 6774 bytes to preference network.proxy.autoconfig_url. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes."

This causes Firefox to fail to contact the mochitest web server via the proxy and causes the test to fail.

Changing 

-// Actually, 4kb should be enough for everyone.
-static const uint32_t MAX_ADVISABLE_PREF_LENGTH = 4 * 1024;
+// Actually, 10kb should be enough for everyone.
+static const uint32_t MAX_ADVISABLE_PREF_LENGTH = 10 * 1024;

works for me.

gbrown: I don't see these errors on the android emulator mda tests. Can you think of any reason why your pac url is short enough to not trigger the warning?

Comment 1

[Geoff Brown [:gbrown]]

Nothing comes to mind. Running locally against the emulator I normally see a length of about 6K, and I don't see the warning. Quick check on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f75196b717ea9b9c29bacc6d84c687b920e1a82

Comment 2

[Geoff Brown [:gbrown]]

(In reply to Geoff Brown [:gbrown] from comment #1)
> Quick check on try:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8f75196b717ea9b9c29bacc6d84c687b920e1a82

It looks like the length of network.proxy.autoconfig_url is 6766 characters:

https://public-artifacts.taskcluster.net/Ae31T-EUTcKr54ct6W5cgQ/0/public/logs/live_backing.log

[task 2018-02-12T17:59:43.426Z] 17:59:43     INFO -  ZZZ RAW len 6766

but I do not see that warning. What fun!

Comment 3

[Bob Clary [:bc] (inactive)]

I do have the log buffer size set to 16M on my local test device so maybe we are just losing the message before it can be written out. I wonder why I see proxy errors but you do not.

Comment 4

[Nicholas Nethercote [inactive]]

> Changing 
> 
> -// Actually, 4kb should be enough for everyone.
> -static const uint32_t MAX_ADVISABLE_PREF_LENGTH = 4 * 1024;
> +// Actually, 10kb should be enough for everyone.
> +static const uint32_t MAX_ADVISABLE_PREF_LENGTH = 10 * 1024;
> 
> works for me.

That makes sense, but this limit has been in place for a long time (it predates bug 1407112) and for good reason. libpref gets abused in a ton of different ways, and storing excessively large strings is one of them :(

Why is network.proxy.autoconfig_url so large in this case?

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: network.proxy.autoconfig_url

I don't think it is actually limited to this one test. It appears the proxy autoconfig url contains all of the different hostnames that are used in any test which combined with the javascript makes this a large value for a pref.

Comment 6

[Bob Clary [:bc] (inactive)]

bug 1617634 may make this obsolete but either way there is no interest... > wontfix.