63451 - Move getCertificatePrincipal() from nsIZipReader to nsIJAR

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Samir Gehani]

Resurrect nsIJAR.  Make nsJAR inherit from nsIJAR and nsIZipRedaer.  Move 
getCertificatePrincipal() from nsIZipReader to nsIJAR.  The original intent was 
to abstract non-zip reader functionality (specifically, security functionality) 
into a separate interface for a modular design.

Comment 1

[Mitchell Stoltz (not reading bugmail)]

will do...

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - please review

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - please review

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Attachment: aarg - stupid mistake in the patch. Please use this one instead.

Comment 5

[Samir Gehani]

3rd patch looks good.  Minor comments follow.

1> Add nsIJAR.idl to libjar/MANIFEST_IDL
2> Add nsIJAR.idl to libjar/makefile.win
3> Move the certtificate-related constants (NOT_SIGNED et. al.) to nsIJAR.idl.

Please build on Win and Mac as well and run an XPInstall sanity test.  Thanks.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Attachment: Final and complete patch. Already got r=sgehani, need sr

Comment 7

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

-  nsJAR* zip = (nsJAR*)mZips.Get(&key); // AddRefs
+  nsJAR* zip = (nsJAR*)(nsIZipReader*)mZips.Get(&key); // AddRefs

Can you use one of the NS_*_CAST macros here and in the other like places?

I don't understand why this is in the diff:

+
/*
     JS_ReportError(aCx, "access disallowed from scripts at %s to documents "
                         "at another domain", str);
-    nsCRT::free(str);
+    */
+
nsCRT::free(str);
+
printf("Failed.\n");

but I sure don't like the naked printf.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch with Shaver's changes

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Contingent on you _not_ removing Pierre from the contributors list, sr=shaver.

Comment 10

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 11

[ckritzer (gone)]

Marking VERIFIED FIXED, per mstoltz's comments