662693 - nsPrincipal should QI its member nsIURIs to nsISupports before (de-)serializing them

Status: NEW

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

IIUC, part of the problem in bug 662242 is that we're serializing a nsIURI *as a nsIURI*, when we should be serializing it as a nsISupports.

The problem is that old builds begin their serializing using the (old) nsIURI IID, which new builds don't understand when de-serializing.

If instead we QI to nsISupports before serializing, then (as long as nsISupports IID doesn't change) we'll still be able to de-serialize, regardless of the nsIURI IID.

So, I think we need this change to prevent more instances of bug 662242 when the nsIURI IID next gets changed...

Comment 1

[Daniel Holbert [:dholbert]]

See bug 524245 for another area of code where we applied this same sort of fix, a while back.

Comment 2

[Masatoshi Kimura [:emk]]

> If instead we QI to nsISupports before serializing, then (as long as nsISupports 
> IID doesn't change)
The nsISupports IID is going to be changed by bug 391275...

Comment 3

[Boris Zbarsky [:bzbarsky]]

At which point sessionstore is well and truly screwed for the update across that change, at least for anything that involves principals...

Unless, of course, we hardcode the old nsISupports IID in the object stream code in the process.

Comment 4

[Boris Zbarsky [:bzbarsky]]

And I must note that serializing the nsISupports IID is pretty silly.  Wish the code were not doing that.  :(

Comment 5

[Benjamin Smedberg]

For persistent datastores we should be serializing it as "nsIURI" not "IID to any particular interface", and make sure that deserialization works across the versions we care about. No?

Comment 6

[Boris Zbarsky [:bzbarsky]]

In my ideal world, yes.  That's not how the object output stream works right now....  :(

Comment 7

[(no longer active)]

So, what kinds of objects do we serialize here?  Relying on IIDs not changing is really fragile...

Can you please point me to the code responsible here too?  This area is new to me...

Comment 8

[Boris Zbarsky [:bzbarsky]]

"Here" as in session restore?

I believe we serialize nsIPrincipal objects, which transitively involves serializing nsIURI objects.  Both have several different concrete implementations.

The relevant code that's most likely to be an issue here is nsPrincipal::Write, nsBinaryOutputStream::WriteCompoundObject, and nsBinaryInputStream::ReadObject.

Note that this last already has some hackery to deal with nsIURI issues, which is precisely what this bug is about.

Comment 9

[(no longer active)]

Hmm, well, hacking around this kind of stuff seems to have precedent :(  I don't have much to add to comment 5, but I'm afraid we might need to hack around this for now.  But that makes me die a little bit inside.