1349987 - nsExtProtocolChannel doesn't initialize its load flags to 0

Status: RESOLVED FIXED

(Firefox :: File Handling, defect)

Comment 1

[Honza Bambas (:mayhemer)]

Attachment: v1 (just init in ctor)

Jason, this is a super simple patch blocking the security check URI bug.

Comment 2

[Honza Bambas (:mayhemer)]

Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

Let's see what will bubble up.

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

Ehm... https://treeherder.mozilla.org/#/jobs?repo=try&revision=74bfb88e87d79150d376b644809103d915dc58df

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/093d0c594fdc
Init nsExtProtocolChannel.mLoadFlags to 0, r=jduell

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/093d0c594fdc

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Looks like a low-risk belts and suspenders patch. Worth Aurora/Beta/ESR52 approval requests?

Comment 7

[Honza Bambas (:mayhemer)]

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

Comment 8

[Gerry Chang [:gchang]]

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-.

Comment 10

[Gerry Chang [: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+.

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 8850597 [details] [diff] [review]
v1 (just init in ctor)

add missing initialization in ctor, esr52+

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1b675b18491d

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5ab83d9424cd

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/3282e8f6a121

Comment 15

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Honza's assessment on manual testing needs (see Comment 7).