nsFilePicker::Show uninitialised use of mAllowURLs

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

Trunk
All
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

I saw this when testing creating a PDF in fennec:

Saving a PDF to disk, looks like we don't call AppendFilter.


==22492== Conditional jump or move depends on uninitialised value(s)
==22492==    at 0x6B0E51C: nsFilePicker::Show(short*) (nsFilePicker.cpp:453)
==22492==    by 0x6CA7E28: NS_InvokeByIndex_P (xptcinvoke_x86_64_unix.cpp:208)
==22492==    by 0x690628E: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (xpcwrappednative.cpp:3054)
==22492==    by 0x690BD60: XPC_WN_CallMethod(JSContext*, unsigned int, unsigned long*) (xpcwrappednativejsops.cpp:1631)
==22492==    by 0x6F8F50D: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (jscntxtinlines.h:684)
==22492==    by 0x6DC9008: js::Invoke(JSContext*, js::CallArgs const&, unsigned int) (jsinterp.cpp:665)
==22492==    by 0x6DC9967: js::ExternalInvoke(JSContext*, js::Value const&, js::Value const&, unsigned int, js::Value*, js::Value*) (jsinterp.cpp:881)
==22492==    by 0x6D5A6B0: JS_CallFunctionValue (jsinterp.h:954)
==22492==    by 0x6676D5F: nsJSContext::CallEventHandler(nsISupports*, void*, void*, nsIArray*, nsIVariant**) (nsJSEnvironment.cpp:2171)
==22492==    by 0x66B4CE9: nsJSEventListener::HandleEvent(nsIDOMEvent*) (nsJSEventListener.cpp:228)
==22492==    by 0x657DE5C: nsEventListenerManager::HandleEventSubType(nsListenerStruct*, nsIDOMEventListener*, nsIDOMEvent*, nsPIDOMEventTarget*, unsigned int, nsCxPusher*) (nsEventListenerManager.cpp:1112)
==22492==    by 0x657E1F8: nsEventListenerManager::HandleEventInternal(nsPresContext*, nsEvent*, nsIDOMEvent**, nsPIDOMEventTarget*, unsigned int, nsEventStatus*, nsCxPusher*) (nsEventListenerManager.cpp:1208)
==22492==
Posted patch patch v.1Splinter Review
Regression from: 

http://hg.mozilla.org/mozilla-central/rev/61b96b507f7c
Assignee: nobody → doug.turner
Attachment #487416 - Flags: review?
Attachment #487416 - Flags: review? → review?(timeless)
Comment on attachment 487416 [details] [diff] [review]
patch v.1

glanced over this at dougt's request -- looks fine to me.

AFAICT, we only would want to allow URLs in our FilePicker if it were specifically requested via AppendFilter().  So if AppendFilter() never gets called (as is the case here, per comment 0), then we *don't* want to allow URLs.  

So PR_FALSE makes sense as a default value for mAllowURLs.

r=dholbert, but you might technically need karl or roc (widget GTK owner/peer) to rubber-stamp this.
Attachment #487416 - Flags: review?(timeless) → review+
Attachment #487416 - Flags: review?(roc)
Hardware: x86_64 → All
Version: unspecified → Trunk
Comment on attachment 487416 [details] [diff] [review]
patch v.1

low risk - just sets default value to a valid state.
Attachment #487416 - Flags: approval2.0?
Attachment #487416 - Flags: approval2.0? → approval2.0+
https://bugzilla.mozilla.org/show_bug.cgi?id=608818
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In comment 4, dougt meant to quote the checkin:)
http://hg.mozilla.org/mozilla-central/rev/b8024b980248

Sadly, this patch was responsible for a silly build warning:
> gtk2/nsFilePicker.h: In constructor ‘nsFilePicker::nsFilePicker()’:
> gtk2/nsFilePicker.h:86: warning: ‘nsFilePicker::mAllowURLs’ will be initialized after
> gtk2/nsFilePicker.h:85: warning:   ‘PRInt16 nsFilePicker::mSelectedType’
> gtk2/nsFilePicker.cpp:195: warning:   when initialized here

(sorry for not having caught that in review)

Attaching trivial followup to fix this warning.
Attachment #506325 - Flags: review?(doug.turner)
Attachment #506325 - Flags: approval2.0?
Attachment #506325 - Attachment description: trivial followup to fix build warning → trivial followup to fix constructor init order
Attachment #506325 - Flags: review?(doug.turner)
Attachment #506325 - Flags: review+
Attachment #506325 - Flags: approval2.0?
Attachment #506325 - Flags: approval2.0+
You need to log in before you can comment on or make changes to this bug.