1323843 - add test for beConservative option added in bug 1321783

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Bug 1321783 added nsIHttpChannelInternal.beConservative as a way to make a channel use conservative TLS settings when connecting. It took a while, but I managed to get together a test for this feature, so I'm adding it now.

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: bug 1323843 - add a test for conservative TLS option added in bug 1321783

Review commit: https://reviewboard.mozilla.org/r/98926/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98926/

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8819062 [details]
bug 1323843 - add a test for conservative TLS option added in bug 1321783

https://reviewboard.mozilla.org/r/98926/#review99228

really appreciate this. thanks.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Thanks!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a86f3e109eef082058a42d71533097bab3eecb56

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5182a866ae5a
add a test for conservative TLS option added in bug 1321783 r=mcmanus

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5182a866ae5a

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8819062 [details]
bug 1323843 - add a test for conservative TLS option added in bug 1321783

Approval Request Comment
[Feature/Bug causing the regression]: n/a (this is just a test for the feature added in bug 1321783)
[User impact if declined]: no test coverage for feature
[Is this code covered by automated tests?]: it is the test
[Has the fix been verified in Nightly?]: not by QA, no (although, again, this is the test)
[Needs manual test from QE? If yes, steps to reproduce]: probably not
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: it's just a test
[String changes made/needed]: none

Comment 7

[Julien Cristau [:jcristau]]

This can go in with a=test-only.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/75a0d7ccab1c

Comment 9

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Oh, right - of course. Thanks!
In the meantime, bug 1321783 got uplifted to beta, so we need this there as well.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/31a671ef6c6f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Disabled on Beta for permafailing. Looks like the test depends on bug 1286694.
https://hg.mozilla.org/releases/mozilla-beta/rev/5df4fe2cddb45311fa3e8ac0dc58f04364ffffb9

https://treeherder.mozilla.org/logviewer.html#?job_id=67034087&repo=mozilla-beta

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Thanks for tracking that down. It looks like I'm a bit late to uplift that and re-enable this test, since the tree is now closed for uplift week (I would have done it yesterday with a=test-only, but claiming that the relevant patch from bug 1286694 is test-only is a slight stretch).

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Code freeze for 51 isn't until next week.

Comment 14

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Oh - I must be misunderstanding the tree status annotation. In any case, I went ahead and asked for uplift for the part 2 patch in bug 1286694 (since part 1 landed in 50). After that lands, we should be able to just back out 5df4fe2cddb45311fa3e8ac0dc58f04364ffffb9.

Comment 15

[Dana Keeler (she/her) (use needinfo) [:keeler]]

https://hg.mozilla.org/releases/mozilla-beta/rev/87cdc832e054458169a65c54fdde29a83faae925