Closed Bug 1446571 Opened 4 years ago Closed 4 years ago

Remove support for update.rdf signing

Categories

(Toolkit :: Add-ons Manager, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

Details

Attachments

(3 files)

This feature is essentially dead code, and maintaining tests for it is more trouble than it's worth. WebExtensions don't support specifying an update key, legacy extensions are no longer supported, and the update.rdf format is deprecated, in any case. It's time to remove this code.
Comment on attachment 8959739 [details]
Bug 1446571: Part 2b - Update update tests to run sequentially.

https://reviewboard.mozilla.org/r/228590/#review234382


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: toolkit/components/extensions/test/xpcshell/head.js:39
(Diff revision 1)
> -      server.stop(resolve);
> -    });
> -  });
> -
> -  return server;
>  }

Error: Missing semicolon. [eslint: semi]
Comment on attachment 8959738 [details]
Bug 1446571: Part 2a - Stop throwing raw numeric values in https.js.

https://reviewboard.mozilla.org/r/228588/#review234474
Attachment #8959738 - Flags: review?(aswan) → review+
Comment on attachment 8959739 [details]
Bug 1446571: Part 2b - Update update tests to run sequentially.

https://reviewboard.mozilla.org/r/228590/#review234476

nice, thank you!
Attachment #8959739 - Flags: review?(aswan) → review+
Attachment #8959737 - Flags: review?(aswan)
Comment on attachment 8959739 [details]
Bug 1446571: Part 2b - Update update tests to run sequentially.

https://reviewboard.mozilla.org/r/228590/#review234666

::: commit-message-42b43:1
(Diff revision 1)
> +Bug 1446571: Part 2b - Update update tests to run sequentially. r?aswan

This description is wrong
Comment on attachment 8959739 [details]
Bug 1446571: Part 2b - Update update tests to run sequentially.

https://reviewboard.mozilla.org/r/228590/#review234666

> This description is wrong

Yes, I noticed :p
Comment on attachment 8959737 [details]
Bug 1446571: Part 1 - Remove support for signed update.rdf files.

https://reviewboard.mozilla.org/r/228586/#review234676

::: toolkit/mozapps/extensions/test/xpcshell/test_updatecheck.js:51
(Diff revision 1)
>  /*
>   * Tests that the security checks are applied correctly
>   *
>   * Test     signature   updateHash  updateLink   expected
>   *--------------------------------------------------------
>   * 2        absent      absent      http         fail
>   * 3        broken      absent      http         fail
>   * 4        correct     absent      http         no update
>   * 5        correct     sha1        http         update
>   * 6        corrent     absent      https        update
>   * 7        corrent     sha1        https        update
>   * 8        corrent     md2         http         no update
>   * 9        corrent     md2         https        update
>   */

Can we replace all the non-https test cases with a single "update fails if the update link is not https" test and get rid of a bunch of associated cruft
Comment on attachment 8959737 [details]
Bug 1446571: Part 1 - Remove support for signed update.rdf files.

https://reviewboard.mozilla.org/r/228586/#review234676

> Can we replace all the non-https test cases with a single "update fails if the update link is not https" test and get rid of a bunch of associated cruft

Yes, but I was planning to do that as part of a larger refactor to all of the update tests when we remove support for non-restartless extensions. These simplifications just give us an easier starting point for that work.
Comment on attachment 8959737 [details]
Bug 1446571: Part 1 - Remove support for signed update.rdf files.

https://reviewboard.mozilla.org/r/228586/#review234676

> Yes, but I was planning to do that as part of a larger refactor to all of the update tests when we remove support for non-restartless extensions. These simplifications just give us an easier starting point for that work.

works for me
Comment on attachment 8959737 [details]
Bug 1446571: Part 1 - Remove support for signed update.rdf files.

https://reviewboard.mozilla.org/r/228586/#review234716
Attachment #8959737 - Flags: review?(aswan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cfea8df6eaef3e3830c533d9d3c3c52e4147630
Bug 1446571: Follow-up: Fix test that expected raw numeric errors. r=bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/afbc519c38563f44f66949df98706ebf4bf13555
Bug 1446571: Follow-up: Fix tests that try to initialize AddonTestUtils twice. r=bustage DONTBUILD
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2d1f86caeaf0a7dd9356bae8d5d6fe059d29c30
Bug 1446571: Follow-up: Fix sync tests that misuse resources from add-on manager tests. r=bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/a95c9add03af42a289d00dda46bb021f51930254
Bug 1446571: Follow-up: Fix another test that expected raw numeric errors. r=bustage DONTBUILD
Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!
Flags: needinfo?(kmaglione+bmo)
This is just a removal of code that hasn't been usable for a long time.
Flags: needinfo?(kmaglione+bmo) → qe-verify-
Blocks: 1453835
You need to log in before you can comment on or make changes to this bug.