Closed Bug 1349987 Opened 3 years ago Closed 3 years ago

nsExtProtocolChannel doesn't initialize its load flags to 0

Categories

(Firefox :: File Handling, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox52 --- wontfix
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file)

No description provided.
Jason, this is a super simple patch blocking the security check URI bug.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8850597 - Flags: review?(jduell.mcbugs)
Blocks: 1319111
Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

Let's see what will bubble up.
Attachment #8850597 - Flags: review?(jduell.mcbugs) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/093d0c594fdc
Init nsExtProtocolChannel.mLoadFlags to 0, r=jduell
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/093d0c594fdc
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Looks like a low-risk belts and suspenders patch. Worth Aurora/Beta/ESR52 approval requests?
Flags: needinfo?(honzab.moz)
Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: unknown
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): low (might expose some other bug where consumers of the load flags expect a flag be set, tho)
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: since ever?
[User impact if declined]: unknown
[Is this code covered by automated tests?]: not sure
[Has the fix been verified in Nightly?]: looks like
[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?]: it might expose a different bug of load flags consumers, but it's a low probability
[Why is the change risky/not risky?]: can't say
[String changes made/needed]: none
Flags: needinfo?(honzab.moz)
Attachment #8850597 - Flags: approval-mozilla-esr52?
Attachment #8850597 - Flags: approval-mozilla-beta?
Attachment #8850597 - Flags: approval-mozilla-aurora?
Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

Since the user impact is unknown and there is a low possibility to expose a different bug of load flags consumers. I prefer to let it ride the train on 55.
Aurora54-, Beta53- & ESR52-.
Attachment #8850597 - Flags: approval-mozilla-esr52?
Attachment #8850597 - Flags: approval-mozilla-esr52-
Attachment #8850597 - Flags: approval-mozilla-beta?
Attachment #8850597 - Flags: approval-mozilla-beta-
Attachment #8850597 - Flags: approval-mozilla-aurora?
Attachment #8850597 - Flags: approval-mozilla-aurora-
Flags: needinfo?(gchang)
Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

Hi Ryan, thanks for the reminder. We need to take this. Aurora54+ & Beta53+.
Flags: needinfo?(gchang)
Attachment #8850597 - Flags: approval-mozilla-esr52?
Attachment #8850597 - Flags: approval-mozilla-esr52-
Attachment #8850597 - Flags: approval-mozilla-beta-
Attachment #8850597 - Flags: approval-mozilla-beta+
Attachment #8850597 - Flags: approval-mozilla-aurora-
Attachment #8850597 - Flags: approval-mozilla-aurora+
Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

add missing initialization in ctor, esr52+
Attachment #8850597 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Setting qe-verify- based on Honza's assessment on manual testing needs (see Comment 7).
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.