Move getCertificatePrincipal() from nsIZipReader to nsIJAR

VERIFIED FIXED

Status

()

VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: samir_bugzilla, Assigned: security-bugs)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

18 years ago
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.
will do...
Status: NEW → ASSIGNED
Created attachment 22076 [details] [diff] [review]
Patch - please review
Created attachment 22077 [details] [diff] [review]
Patch - please review
Created attachment 22098 [details] [diff] [review]
aarg - stupid mistake in the patch. Please use this one instead.
(Reporter)

Comment 5

18 years ago
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.
Created attachment 25631 [details] [diff] [review]
Final and complete patch. Already got r=sgehani, need sr
-  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.
Created attachment 25741 [details] [diff] [review]
Patch with Shaver's changes
Contingent on you _not_ removing Pierre from the contributors list, sr=shaver.
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 11

18 years ago
Marking VERIFIED FIXED, per mstoltz's comments
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.