Closed Bug 1424915 Opened 2 years ago Closed 2 years ago

Restarting a nsHttpTransaction can cause a transaction to be added twice to the transaction pending queue

Categories

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

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

This is follow up for bug 1423988.

The explanation what is exactly happening can be found in bug 1423988 comment 21.
Attachment #8936656 - Flags: review?(mcmanus)
Attachment #8936656 - Flags: review?(mcmanus) → review+
is this too late for 58?
(In reply to Patrick McManus [:mcmanus] from comment #2)
> is this too late for 58?

I do not know I will try.
Blocks: 1425196
Pushed by dd.mozilla@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7545a771197d
Close a httpTransaction from a http2session with the right error code if the error code is NS_BINDING_RETARGETED. Otherwise the transaction will be restarted twice. r=mcmanus
https://hg.mozilla.org/mozilla-central/rev/7545a771197d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8936656 [details] [diff] [review]
bug_1424915_v1.patch

Approval Request Comment
[Feature/Bug causing the regression]: A long existing bug, probably introduced by bug 1003448
[User impact if declined]: The bug can be triggered with a 421 or a 425 response and http2 protocol. The 425 response is decoded only if TLS1.3 early data is turned on. With TLS1.3 early-data turned on it causes crash like bug 1423988. Without TLS1.3 turned on the bug can be triggered with a 421 response when the http2 protocol is used. When the bug is triggered a http transaction can get in a strange state that may cause a crash as well. I would not be surprise that some crashes are connected with this bug. These crashes will be hard to analyze and debug.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: Yes. the patch is in Nightly, TLS1.3 is turn on again on Nightly and crash from bug 1423988 did not appear again.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: Very low risk. I tested it manually. It is very isolated change (only a single error is propagated properly through Http2Session code). 
[String changes made/needed]:none
Attachment #8936656 - Flags: approval-mozilla-beta?
Comment on attachment 8936656 [details] [diff] [review]
bug_1424915_v1.patch

Avoid a potential crash related to http2 protocol. Beta58+.
Attachment #8936656 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.