571619 - Can't QueryInterface from the canonical nsISupports pointer of an nsSimpleNestedURI directly to nsINestedURI

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[[e-mail]]

nsSimpleNestedURI inherits from nsSimpleURI, which supports aggregation. It does this by containing a dummy object that performs QueryInterface on its behalf. Thus whenever you QueryInterface to an interface that nsSimpleURI knows about, you get the nsSimpleURI itself, otherwise you get whatever the aggregate or dummy object supports.

Because the dummy object only supports nsISupports, it cannot QueryInterface to nsINestedURI. Only the real nsSimpleNestedURI object can do that.

There appear to be three possibilities:
1. Make nsSimpleNestedURI an object that aggregates nsSimpleURI. Looking at the // XXX comment in StartClone, this would probably break serialisation.
2. Remove the aggregation code from nsSimpleURI. This means that it gets a normal QueryInterface method that nsSimpleNestedURI can inherit.
3. Write a custom QueryInterface method for nsSimpleNestedURI that hides the aggregation. It can do this either by implementing all the interfaces that nsSimpleNestedURI supports or by forwarding all the interfaces except nsISupports and nsINestedURI to nsSimpleURI.

Comment 1

[[e-mail]]

Attachment: Possible patch

This is one way of implementing option 3. Another way would be to write
NS_IMPL_QUERY_INTERFACE5(nsSimpleNestedURI, nsIURI, nsISerializable, nsIClassInfo, nsIMutable, nsINestedURI)

Comment 2

[Boris Zbarsky [:bzbarsky]]

That sems fine...  I'd also be fine with solution 2, since I've never understood why aggregation is involved here at all.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

OK, after some mxr'ing the aggregation part of nsSimpleURI seems unused, so it's probably best to just remove it and go with option 2.

Comment 4

[[e-mail]]

Attachment: With test

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 450860 [details] [diff] [review]
With test

-    } else if (aIID.Equals(kThisSimpleURIImplementationCID) || // used by Equals

you need to keep this, right?

Comment 6

[[e-mail]]

Attachment: Updated for review comments

Comment 7

[[e-mail]]

Pushed changeset 1b9e95be69ab to mozilla-central.