First seen: Sat Sep 30, 2017, 1:48:57 TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_cert_blocklist.js | xpcshell return code: 0 [log…] TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_cert_blocklist.js | - MIGKMQswCQYDVQQGEwJDSDEQMA4GA1UEChMHV0lTZUtleTEmMCQGA1UECxMdQ29weXJpZ2h0IChjKSAyMDA1IFdJU2VLZXkgU0ExFjAUBgNVBAsTDUludGVybmF0aW9uYWwxKTAnBgNVBAMTIFdJU2VLZXkgQ2VydGlmeUlEIEFkdmFuY2VkIEcxIENB should be an expected top-level line in revocations.txt - false == true [log…] M-C last good: cd9c8c48e4b3ded47a776f757008f3dcf5 M-C first bad: 57f68296c350469d73d788eb3695a89894 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd9c8c48e4b3ded47a776f757008f3dcf5&tochange=57f68296c350469d73d788eb3695a89894 Seems to be related to bug 1359428. Mark, David, Mathieu, Robert, could you please advise me what I need to do so this test doesn't fail in the Thunderbird Xpcshell suite.

It looks like the test is picking up the real revocations.txt file? (Although I don't understand how it could do that since I thought tests weren't allowed to connect to external resources, although maybe if we're serving it from the same data center it wouldn't consider it external?) Maybe the test should set the "services.settings.server"/"extensions.blocklist.url" prefs earlier than it does?

David, thanks for the reply, but this is really far removed from my area of expertise. Can the test be changed to pass for TB or should we disable it?

This failing assertion does not tell us if the resulting revocations.txt is empty or contains other values. You can check around that, and I'll make my best to help you... Basically, in Bug 1359428 we removed the pref `security.onecrl.via.amo` which is now always false. The tests consists in: - triggering a blocklist sync from a fake remote kinto server (which executes a specific blocklist callback that notifies the nsICertBlocklist of the changes) - making sure the revocation.txt generated by nsICertBlocklist service matches the fake entries I don't have any strong idea about what could be causing this :( Marc, any guess?

I can run that test locally mozilla/mach xpcshell-test security/manager/ssl/tests/unit/test_cert_blocklist.js and I get: 0:13.52 TEST_STATUS: Thread-1 FAIL MIGKMQswCQYDVQQGEwJDSDEQMA4GA1UEChMHV0lTZUtleTEmMCQGA1UECxMdQ29weXJpZ2h0IChjKSAyMDA1IFdJU2VLZXkgU0ExFjAUBgNVBAsTDUludGVybmF0aW9uYWwxKTAnBgNVBAMTIFdJU2VLZXkgQ2VydGlmeUlEIEFkdmFuY2VkIEcxIENB should be an expected top-level line in revocations.txt - false == true and later on: 0:14.30 PROCESS_OUTPUT: Thread-1 (pid:5572) "JavaScript strict warning: resource://testing-common/httpd.js, line 800: ReferenceError: reference to undefined property "_stopCallback"" 0:14.30 PROCESS_OUTPUT: Thread-1 (pid:5572) "!!! error running onStopped callback: TypeError: callback is not a function" 0:13.55 LOG: Thread-1 INFO "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "_stopCallback"" {file: "resource://testing-common/httpd.js" line: 800}]" Since this is the first time I've run this test manually, I assume this file: Directory of C:\Users\jorgk\AppData\Local\Temp\firefox\xpcshellprofile 03/10/2017 19:31 20,466 revocations.txt (Windows, European time format) originates from the run I've just done. I'll attach it in the next comment. Looking at the message: MIGKMQswCQYDVQQGEwJDSDEQMA4GA1UEChMHV0lTZUtleTEmMCQGA1UECxMdQ29weXJpZ2h0IChjKSAyMDA1IFdJU2VLZXkgU0ExFjAUBgNVBAsTDUludGVybmF0aW9uYWwxKTAnBgNVBAMTIFdJU2VLZXkgQ2VydGlmeUlEIEFkdmFuY2VkIEcxIENB should be an expected top-level line in revocations.txt and at the file, I see: MIGKMQswCQYDVQQGEwJDSDEQMA4GA1UEChMHV0lTZUtleTEmMCQGA1UECxMdQ29weXJpZ2h0IChjKSAyMDA1IFdJU2VLZXkgU0ExFjAUBgNVBAsTDUludGVybmF0aW9uYWwxKTAnBgNVBAMTIFdJU2VLZXkgQ2VydGlmeUlEIEFkdmFuY2VkIEcxIENB as the fourth line of the file. Looking at the test, 'expected' doesn't appear to be set up with that line, so that's why the test fails.

Looking at the revocations.txt file you've provided (thanks for that), we appear to have actual blocklist values (i.e. loaded from services/blocklists/certificates.json), plus some of the values that are added by the test. My recollection is that these aren't typically loaded when we run this test (for Fx, at least). Is there some difference between how this data ends up in the profile data for thunderbird and FX?

(In reply to Mark Goodwin [:mgoodwin] from comment #6) > Is there some difference between how this data ends up in the profile data > for thunderbird and FX? Apparently so ;-) What determines how this data is loaded and how can I debug that? Otherwise the only option I see is to switch off that check for TB.

Mat, is there some magic that stops kinto loading initial data for collections in test profiles for Fx?

While trying to follow comment #1 (quote): > Maybe the test should set the "services.settings.server"/"extensions.blocklist.url" > prefs earlier than it does? I ran the test locally and noticed that things were going wrong before the test failure: 0:09.25 LOG: Thread-1 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "[Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICertBlocklist.revokeCertByIssuerAndSerial]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource://services-common/blocklist-clients.js :: updateCertBlocklist :: line 358" data: no]"] updateCertBlocklist@resource://services-common/blocklist-clients.js:358:9 async*maybeSync/<@resource://services-common/blocklist-clients.js:310:17 async*openCollection@resource://services-common/blocklist-clients.js:140:20 async*maybeSync@resource://services-common/blocklist-clients.js:233:20 async*this.checkVersions@resource://services-common/blocklist-updater.js:164:15 async*notify@file:///c:/mozilla-source/comm-central/obj-i686-pc-mingw32/dist/bin/components/nsBlocklistService.js:638:7 fetch_blocklist/<@c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/security/manager/ssl/tests/unit/test_cert_blocklist.js:225:5 Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:390:5 fetch_blocklist@c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/security/manager/ssl/tests/unit/test_cert_blocklist.js:215:10 run_test/<@c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/security/manager/ssl/tests/unit/test_cert_blocklist.js:357:11 TaskImpl_run@resource://gre/modules/Task.jsm:331:42 TaskImpl@resource://gre/modules/Task.jsm:280:3 asyncFunction@resource://gre/modules/Task.jsm:252:14 Task_spawn@resource://gre/modules/Task.jsm:166:12 _run_next_test@c:\\mozilla-source\\comm-central\\mozilla\\testing\\xpcshell\\head.js:1488:9 run@c:\\mozilla-source\\comm-central\\mozilla\\testing\\xpcshell\\head.js:701:9 _do_main@c:\\mozilla-source\\comm-central\\mozilla\\testing\\xpcshell\\head.js:221:3 _execute_test@c:\\mozilla-source\\comm-central\\mozilla\\testing\\xpcshell\\head.js:544:5 @-e:1:1 " That repeats three times. Perhaps that gives a clue.

What's the value that's causing the error? (see https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/CertBlocklist.cpp#309 - it looks like setting the environment variable "MOZ_LOG=CertBlock:5" should say what it is).

With that logging it got quite wordy. I also added print statements into the function. If you look for "failed on", you see: "=== failed on |MBIxEDAOBgNVBAMMB1Rlc3QgQ0E=| |oops! more nonsense.|" and three more. But that's right in the middle and appears unrelated to the 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICertBlocklist.revokeCertByIssuerAndSerial]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource://services-common/blocklist-clients.js :: updateCertBlocklist :: line 358" data: no]"] Or maybe it's related since there are four "failed on" and also four JS errors. And the whole thing is async, right? Full log enclosed. P.S.: Thanks for the help.

Looks like the LOG: Thread-1 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "[Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICertBlocklist.revokeCertByIssuerAndSerial]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource://services-common/blocklist-clients.js :: updateCertBlocklist :: line 358" data: no]"] are expected since they originate from "=== failed on |MBIxEDAOBgNVBAMMB1Rlc3QgQ0E=| |oops! more nonsense.|" and friends. We're not into the third week of this test busting every single Thunderbird Xpcshell test run which makes sheriffing harder than it should be. In this patch I'm suggesting to just switch off the part that fails since sadly I have received very little support to get to the bottom of the problem.

Comment on attachment 8918986 [details] [diff] [review] 1404577-disable-part-of-test_cert_blocklist.patch (v1) Review of attachment 8918986 [details] [diff] [review]: ----------------------------------------------------------------- If we have to, I'd rather just disable the test on thunderbird. I've been looking into this, and I think the unexpected entries are coming from objdir/dist/bin/defaults/blocklists/certificates.json, which exists when running on comm-central but not mozilla-central. I don't know why that is, though - perhaps Mark does.

Ah - it does exist on mozilla-central, but as objdir/dist/bin/browser/defaults/blocklists/certificates.json (note the extra "browser" directory). I believe this is a result of these lines: if CONFIG['MOZ_BUILD_APP'] == 'browser': DIST_SUBDIR = 'browser' in https://dxr.mozilla.org/mozilla-central/rev/20d57b9c4183973af4af5e078dff2aec0b74f928/services/blocklists/moz.build#17 Removing the conditional and recompiling (and removing the objdir/dist/bin/defaults/blocklists/certificates.json file first, just in case) results in this test succeeding.

Comment on attachment 8918986 [details] [diff] [review] 1404577-disable-part-of-test_cert_blocklist.patch (v1) This patch wasn't really meant seriously, more to express the fact the ongoing failure is rather, well, how to say it nicely, ... unfortunate :-( (In reply to David Keeler [:keeler] (use needinfo?) from comment #14) > Removing the conditional and recompiling (and removing the > objdir/dist/bin/defaults/blocklists/certificates.json file first, just in > case) results in this test succeeding. Wow, \o/, thanks! Could you do a patch for this?

Try run with the enclosed patch based on David's idea/observation: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=76c27356e450037e226ee2542cdce03b18e62b1a

(In reply to Jorg K (GMT+2) from comment #15) > (In reply to David Keeler [:keeler] (use needinfo?) from comment #14) > > Removing the conditional and recompiling (and removing the > > objdir/dist/bin/defaults/blocklists/certificates.json file first, just in > > case) results in this test succeeding. > Wow, \o/, thanks! Could you do a patch for this? Looks like you beat me to it. In any case, I don't really know the effect of doing this - we should get some input from kinto folks.

The previous patch didn't work, but this one does. The JSON file gets put into dist\bin\mail\defaults\blocklists\certificates.json and the test passes. Here's another try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1bf7d8faf18d2dd22b48d7b437835d46f66fb6dc David, please pass on the review as you deem fit. We're into the third week of having our xpcshell runs busted due to this, so quick action would be appreciated.

This also needs a C-C change. New try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6d02df0f32a4a472c3cf5c06aa925ab72474b4b6

Comment on attachment 8920310 [details] [diff] [review] 1404577-blocklist-test.patch (v2) Review of attachment 8920310 [details] [diff] [review]: ----------------------------------------------------------------- Redirecting to original author.

Again: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=039e0620dd10e22b82b889d86b596d4d8792a399 Or maybe Tom has a better idea.

This looks reasonable. I don't understand how the blocklist code works, or what the implications of putting the files in a different location (particularly as this is security sensitive code). The m-c patch will need to be ajusted to take into account building c-c with m-c as topdir: MOZ_BUILD_APP will be `comm/mail` in that case. (so maybe `CONFIG["MOZ_BUILD_APP"] in ("../mail", "comm/mail")`?).

OK, taking Tom's suggestions on board.

I was talking with people in #build and I think this is the wrong approach. firefox has two omni.ja bundles, on in the root and one in browser/. They can be accessed as resource://gre/ and resource://app/ respectively. Thunderbird, on the other hand, only has one. I think this change may "work" because the loading code can no longer find the bundled blocklist, but this break the actual desired behavior while making the test pass.

I'd say it doesn't matter where the files are put as long as they are packaged. This time my try https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=039e0620dd10e22b82b889d86b596d4d8792a399 at least built, the Xpcshell test hasn't started yet. I'm happy for someone else to take the bug and fix it. I've tried for three(!!) weeks so far to get a bustage fixed which affects every single xpcshell test we run on all platforms. So far I've received little support apart from David's who observed that the test fails if the file is placed into dist\bin\defaults\blocklists\certificates.json since FF puts it into dist\bin\browser\defaults\blocklists\certificates.json so I'm trying to imitate that and put it into dist\bin\mail\defaults\blocklists\certificates.json. I don't understand why the test suddenly started failing after https://hg.mozilla.org/mozilla-central/rev/a889835e4003 which didn't change any make/build files. I also don't understand why the test tries to pick up a file that in the case of FF isn't there. Instead of moving the file out of the way, IMHO it would be preferable not to go looking for it in the first place.

(In reply to Jorg K (GMT+2) from comment #25) > I'd say it doesn't matter where the files are put as long as they are > packaged. Of course it matters. Unless files are where the code expects them to be, they can't be accessed by that code. And if they can't be accessed, they might as well not be packaed. > I don't understand why the test suddenly started failing after > https://hg.mozilla.org/mozilla-central/rev/a889835e4003 which didn't change > any make/build files. Looking at that commit, it changes that test file. Before that commit, the test was using a deprecated method of updating the blocklist that had been disabled for sometime. That commit got rid of the old method of updating the blocklist and adjusted the test to use the new method. > I also don't understand why the test tries to pick up a file that in the > case of FF isn't there. Instead of moving the file out of the way, IMHO it > would be preferable not to go looking for it in the first place. It turns out this hinges on a difference between firefox vs. xpcshell and thunderbird: - In firefox, dist/bin/browser/ is available as resource://app/ - In xpcshell and firefox, dist/bin/ is available as resource://app/ So, in a firefox build, resource:://app/defaults/blocklists/certificates.json exists when running the app, but doesn't in tests. Whereas in thunderbird builds, it exists in both the app and the tests. It looks like there was some[1][2] attempt to avoid loading the blocklist dump, but since it didn't cause the firefox tests to fail, it never ended up being implemented. (Though there was a comment[3] asking if that was leftover code) [1] https://hg.mozilla.org/mozilla-central/rev/a889835e4003#l5.12 [2] https://hg.mozilla.org/mozilla-central/rev/a889835e4003#l3.254 [3] https://reviewboard.mozilla.org/r/176422/#review181706 I think the correct solution is to honor that preference.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=26262374b030671b9d13bbdb65beea54232d7f98 https://treeherder.mozilla.org/#/jobs?repo=try&revision=82459dcbf6239298c6fafb329897f329dce267fe

Comment on attachment 8920363 [details] [diff] [review] 1404577-blocklists-packaging.patch (v2) Thanks Tom!

Comment on attachment 8920456 [details] Bug 1404577: Actually disable loading blocklist dump in test_cert_blocklist.js. https://reviewboard.mozilla.org/r/191442/#review196704

Ryan, could you please land this.

Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/autoland/rev/0b2f0f688fe3 Actually disable loading blocklist dump in test_cert_blocklist.js. r=leplatrem