Move getCertificatePrincipal() from nsIZipReader to nsIJAR

VERIFIED FIXED

Status

()

Core
Security
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Samir Gehani, Assigned: Mitchell Stoltz (not reading bugmail))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

17 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.
(Assignee)

Comment 1

17 years ago
will do...
Status: NEW → ASSIGNED
(Assignee)

Comment 2

17 years ago
Created attachment 22076 [details] [diff] [review]
Patch - please review
(Assignee)

Comment 3

17 years ago
Created attachment 22077 [details] [diff] [review]
Patch - please review
(Assignee)

Comment 4

17 years ago
Created attachment 22098 [details] [diff] [review]
aarg - stupid mistake in the patch. Please use this one instead.
(Reporter)

Comment 5

17 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.
(Assignee)

Comment 6

17 years ago
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.
(Assignee)

Comment 8

17 years ago
Created attachment 25741 [details] [diff] [review]
Patch with Shaver's changes
Contingent on you _not_ removing Pierre from the contributors list, sr=shaver.
(Assignee)

Comment 10

17 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 11

17 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.