Closed Bug 1563695 Opened 5 years ago Closed 5 years ago

Do not close SpdyConnectTransaction in TLSFilterTransaction

Categories

(Core :: Networking: HTTP, defect, P1)

68 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla69
Tracking Status
firefox-esr68 68+ fixed
firefox68 + fixed
firefox69 --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [necko-triaged][secure-proxy-mvp])

Attachments

(1 file)

This is a regression from bug 1484947.

Pushed by honzab.moz@firemni.cz:
https://hg.mozilla.org/integration/autoland/rev/7d95f38f8dad
Do not close SpdyConnectTransaction in TLSFilterTransaction. r=mayhemer
Blocks: secure-proxy
Whiteboard: [necko-triaged][secure-proxy-mvp]
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Comment on attachment 9076167 [details]
Do not close SpdyConnectTransaction in TLSFilterTransaction. r=mayhemer

Beta/Release Uplift Approval Request

  • User impact if declined: Https proxy that uses http2 will not work properly. The connection will fill slower than it is.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a (recent) regression. It was introduced by bug 1484947. Before the bug landed this code was running fine for years. This patch just removes remaining part of the code introduced by bug 1484947 (The other part was removed in bug 1563695). There has been no changes to this code that could rely on the code introduced in bug 1484947.
  • String changes made/needed:
Attachment #9076167 - Flags: approval-mozilla-beta?
Blocks: 1484947

You should have asked release approval.

Comment on attachment 9076167 [details]
Do not close SpdyConnectTransaction in TLSFilterTransaction. r=mayhemer

Beta/Release Uplift Approval Request

  • User impact if declined: Https proxy that uses http2 will not work properly. The connection will fill slower than it is.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a (recent) regression. It was introduced by bug 1484947. Before the bug landed this code was running fine for years. This patch just removes remaining part of the code introduced by bug 1484947 (The other part was removed in bug 1563695). There has been no changes to this code that could rely on the code introduced in bug 1484947.
  • String changes made/needed:
Attachment #9076167 - Flags: approval-mozilla-release?

Comment on attachment 9076167 [details]
Do not close SpdyConnectTransaction in TLSFilterTransaction. r=mayhemer

this is already in 69.

Attachment #9076167 - Flags: approval-mozilla-beta? → approval-mozilla-beta-

I will mark this issue qe-verify- as per comment 4 and comment 6.

Flags: qe-verify-

I verified this on 68 using the manual add-on, confirmed that visiting https://expired.badssl.com/ generates the appropriate certificate error message.

(In reply to Rebecca Billings [:rbillings] from comment #9)

I verified this on 68 using the manual add-on, confirmed that visiting https://expired.badssl.com/ generates the appropriate certificate error message.

I don't think this has anything to do with this bug.

Blocks: 1565518

Please add repro steps with expected results. I was going from comment 1, and if that doesn't relate then this needs more info.

Flags: needinfo?(odvarko)
Flags: needinfo?(honzab.moz)

There is no specific set of steps to verify this bug. This bug is complementary to bug 1563538, you can verify them both using the same set of steps. General browsing on reddit, facebook, youtube (feel free to any heavier sites to the list) at the same time should lead to no interruptions, broken or infinite time taking loads.

Flags: needinfo?(honzab.moz)

Testing with 3+ tabs, I am getting consistent [1 or more per view] lags watching youtube videos. The pauses last from a few seconds up to maybe 30s. If there is additional detail that I can add from logs or the console that would be helpful, let me know.

I was using yesterday's build, will update with today's build and re-test.

While testing there was a slightly noticeable load time when skipping through different videos if letting the videos run normally, no issues were noticeable.

Normal browsing didn't suffer much, Facebook, Reddit, Twitter and Youtube working as intended without any kind of hangs or infinite loading times happening, but slower loading of the pages was noticeable in some cases compared to having the proxy turned off.

I tested multiple tabs and browsed through all of the sites above, running videos. I was not getting any consistent lags or delays in video playback or page load.

Status: RESOLVED → VERIFIED

Comment on attachment 9076167 [details]
Do not close SpdyConnectTransaction in TLSFilterTransaction. r=mayhemer

fix for http2 proxies, needed for the secure-proxy project, approved for 68.0.1 and 68.1esr

Attachment #9076167 - Flags: approval-mozilla-release?
Attachment #9076167 - Flags: approval-mozilla-release+
Attachment #9076167 - Flags: approval-mozilla-esr68+

clearing NI for the other Honza.

Flags: needinfo?(odvarko)
Regressed by: 1484947
Keywords: regression

Per discussion with jcristau, we're uplifting this to 68.0.1esr also to maintain parity with the non-ESR 68.0.1 release and hopefully avoid some confusion.

Blocks: 1579064
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: